Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
379483002:
Align web content's layer to toplevel window instead of root on ash (Closed)
|
Created:
6 years, 5 months ago by oshima Modified:
6 years, 5 months ago CC:
chromium-reviews, yusukes+watch_chromium.org, yukishiino+watch_chromium.org, jam, penghuang+watch_chromium.org, sievers+watch_chromium.org, jbauman+watch_chromium.org, nona+watch_chromium.org, darin-cc_chromium.org, kalyank, piman+watch_chromium.org, danakj+watch_chromium.org, James Su, miu+watch_chromium.org, jamesr Base URL:
svn://svn.chromium.org/chrome/trunk/src Project:
chromium Visibility:
Public. |
DescriptionAlign web content's layer to toplevel window instead of root on ash.
On ash, toplevel window (which is a widget) is already aligned so the RWHV should be aligned to the toplevel window intead of RootWindow.
This depends on the following CL:
https://codereview.chromium.org/375693006/
https://codereview.chromium.org/357063002/
BUG=391822
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=283940
Patch Set 1 : #Patch Set 2 : #Patch Set 3 : check snapped != target #Messages
Total messages: 20 (0 generated)
sky -> owner's review, who also reviewed the original CL (https://codereview.chromium.org/366543004) cc'ing calamity who is the author of the original CL.
LGTM One question though. If we adjust to the left/right by a pixel or 2, what happens to the extra column of pixels? Who fills them?
On 2014/07/16 00:03:20, sky wrote: > LGTM > > One question though. If we adjust to the left/right by a pixel or 2, what > happens to the extra column of pixels? Who fills them? The offset will never be larger than one pixel, and the content of the layer behind it will fill the gap. If this ever becomes an issue (say, the layer behind it has a hole that used by be filled by shifted layer), we can fix it by fixing that content.
The CQ bit was checked by oshima@chromium.org
The CQ bit was unchecked by oshima@chromium.org
Might that result in a very visible line down the side? On Wed, Jul 16, 2014 at 11:51 AM, <oshima@chromium.org> wrote: > On 2014/07/16 00:03:20, sky wrote: > >> LGTM >> > > One question though. If we adjust to the left/right by a pixel or 2, what >> happens to the extra column of pixels? Who fills them? >> > > The offset will never be larger than one pixel, and the content of the > layer > behind it will fill the gap. > If this ever becomes an issue (say, the layer behind it has a hole that > used by > be filled by shifted layer), > we can fix it by fixing that content. > > > https://codereview.chromium.org/379483002/ > To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
If there was no gap, there must be no gap even at 1.25/1.5 scale factor. If there was a gap, there must already be a line there. It may become thicker but this is inevitable unless we change the way to handle hidpi. It's possible to change the rounding to std::floor, instead of std::round, so that it always fit to the narrower side, but It may increase the inconsistency with the content of the layer behind it, as the content of the layers are drawn at the target scale factor. On Wed, Jul 16, 2014 at 12:41 PM, Scott Violet <sky@chromium.org> wrote: > Might that result in a very visible line down the side? > > > On Wed, Jul 16, 2014 at 11:51 AM, <oshima@chromium.org> wrote: > >> On 2014/07/16 00:03:20, sky wrote: >> >>> LGTM >>> >> >> One question though. If we adjust to the left/right by a pixel or 2, what >>> happens to the extra column of pixels? Who fills them? >>> >> >> The offset will never be larger than one pixel, and the content of the >> layer >> behind it will fill the gap. >> If this ever becomes an issue (say, the layer behind it has a hole that >> used by >> be filled by shifted layer), >> we can fix it by fixing that content. >> >> >> https://codereview.chromium.org/379483002/ >> > > To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
Would it make sense to round up the size of the content so that content is always filling the space? This could result in a column of pixels being clipped, but IMO is better than a line that is not necessarily drawn as the same color of the page. On Wed, Jul 16, 2014 at 1:15 PM, oshima <oshima@chromium.org> wrote: > If there was no gap, there must be no gap even at 1.25/1.5 scale factor. > If there was a gap, there must already be a line there. > It may become thicker but this is inevitable unless we change the way to > handle hidpi. > It's possible to change the rounding to std::floor, instead of std::round, > so that it always fit to the narrower side, > but It may increase the inconsistency with the content of the layer behind > it, as the content of the layers are drawn at the target scale factor. > > > > On Wed, Jul 16, 2014 at 12:41 PM, Scott Violet <sky@chromium.org> wrote: > >> Might that result in a very visible line down the side? >> >> >> On Wed, Jul 16, 2014 at 11:51 AM, <oshima@chromium.org> wrote: >> >>> On 2014/07/16 00:03:20, sky wrote: >>> >>>> LGTM >>>> >>> >>> One question though. If we adjust to the left/right by a pixel or 2, >>>> what >>>> happens to the extra column of pixels? Who fills them? >>>> >>> >>> The offset will never be larger than one pixel, and the content of the >>> layer >>> behind it will fill the gap. >>> If this ever becomes an issue (say, the layer behind it has a hole that >>> used by >>> be filled by shifted layer), >>> we can fix it by fixing that content. >>> >>> >>> https://codereview.chromium.org/379483002/ >>> >> >> > To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
Oshima set me straight here. This is the right thing to do.
https://codereview.chromium.org/366543004 only moved the method. ccing jamesr@ as the one true author (from https://codereview.chromium.org/304973003). Removing myself since I have no expertise here.
The CQ bit was checked by oshima@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/oshima@chromium.org/379483002/80001
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: linux_chromium_chromeos_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_chromeos_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
The CQ bit was checked by oshima@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/oshima@chromium.org/379483002/120001
lgtm
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: android_dbg_triggered_tests on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...) win_chromium_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)
Message was sent while issue was closed.
Change committed as 283940 |
