[Pixel Canvas] 1px gap at the bottom and right edge of the shelf |
||||||||||
Issue descriptionWhat steps will reproduce the problem? (1) Change device scale factor or zoom level to anything greater than 1. (2) At some values, there is a noticable a 1px gap at the bottom and right edge of the shelf.
,
Jun 22 2018
Issue 855668 has been merged into this issue.
,
Jun 25 2018
,
Jul 2
This seems to happen only when a layer/window goes out of bounds and we can see the extra 1 px that should have otherwise been clipped or masked. We have a SetMasksToBounds() set to true for the root layer. Notice the 1px of the app window not being cropped below (& to the right) of the shelf.
,
Jul 3
Axis aligned clipping is done with a GL_SCISSOR in the target. So in order to not get clipped: - The scissor rect is not what it should be - The layer bounds are not what you think they are - The transformation of the layer bounds into a clip rect are getting rounded up in the target to include partial pixels? - The clip isn't actually being applied to this window for some reason, I don't think we have the ability to escape a parent clip but not sure.
,
Jul 3
Is there someone who can look into this as I am quite unfamiliar in this domain? I can debug and run some initial tests until. Where should I be looking?
,
Jul 3
You can start printing things in GLRenderer to get an idea what coordinates are being used, and compare those to your expectations.
,
Jul 3
I tried looking at the file but I am unable to make any sense of it. Assigning it to you so you can take a look or delegate it to the right person?
,
Jul 3
The scissor clip rect is set here: https://cs.chromium.org/chromium/src/components/viz/service/display/gl_renderer.cc?rcl=98c28a6f258f58f00ef7caec2c8048b886d40df4&l=2894 void GLRenderer::SetScissorTestRect(const gfx::Rect& scissor_rect) { EnsureScissorTestEnabled(); // Don't unnecessarily ask the context to change the scissor, because it // may cause undesired GPU pipeline flushes. if (scissor_rect == scissor_rect_) return; scissor_rect_ = scissor_rect; FlushTextureQuadCache(SHARED_BINDING); gl_->Scissor(scissor_rect.x(), scissor_rect.y(), scissor_rect.width(), scissor_rect.height()); } Does that help? This seems like something your team could look at, as its the ChromeOS UI that's having an issue? I don't think I'll have time to look at it very soon personally.
,
Jul 9
Issue 752676 has been merged into this issue.
,
Jul 16
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c1a826ab79d4303ddd87bbbf4f6d38995c4bcd36 commit c1a826ab79d4303ddd87bbbf4f6d38995c4bcd36 Author: Malay Keshav <malaykeshav@chromium.org> Date: Mon Jul 16 20:19:56 2018 Set correct pixel size for the compositor Right now we clip(GL_SCISSOR) the display buffer based on the physical pixel size of the display. The pixel size we send to the ui compositor is the size we receive from the platform window(the display in Chrome OS). However, this may not be what we want if the platform has some fractional scale applied. UI elements have their sizes set in DIP. If some UI element (ui::Layer) has its size set to the DIP size of the display, their scaled size (the pixel size after applying device scale factor) after rounding may not match the physical pixel size of the display. This difference may lead to 1px lines left unclipped. This happens in the case of the shelf whose width is set to the width of the display. For a device like eve where the internal display has a physical resolution width of 2400px, if a device scale of 1.8 is applied, the DIP width of the display becomes 1333. We set the bounds of the shelf to this. Now when we do the actual paint, we scale the shelf to its physical pixel size which is ROUND(1333*1.8) = 2399. At the same time the compositor will clip things at width 2400. This difference is what gives the shelf a 1px gap. A proper way to fix this is to scale ui::Layer bounds such that it snaps to the parent's or displays edge similar to what pixel canvas does in views::View. However, that is a large change. In the meantime this patch sets the correct physical pixel size for the compositor so that correct clipping happens. Bug: 843354 Change-Id: Ic9e055c0ba39d85a7297809baf946398bf09f40a Component: UI compositor, pixel canvas, scaling, HiDPI Reviewed-on: https://chromium-review.googlesource.com/1130495 Reviewed-by: Mitsuru Oshima <oshima@chromium.org> Reviewed-by: Nico Weber <thakis@chromium.org> Commit-Queue: Malay Keshav <malaykeshav@chromium.org> Cr-Commit-Position: refs/heads/master@{#575402} [modify] https://crrev.com/c1a826ab79d4303ddd87bbbf4f6d38995c4bcd36/ash/host/ash_window_tree_host_platform.cc [modify] https://crrev.com/c1a826ab79d4303ddd87bbbf4f6d38995c4bcd36/ash/host/ash_window_tree_host_platform.h [modify] https://crrev.com/c1a826ab79d4303ddd87bbbf4f6d38995c4bcd36/ui/aura/window_tree_host.cc [modify] https://crrev.com/c1a826ab79d4303ddd87bbbf4f6d38995c4bcd36/ui/aura/window_tree_host.h
,
Jul 16
,
Jul 25
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/48d26c3d8f9f81fe52e33abe390ba235e42b65bf commit 48d26c3d8f9f81fe52e33abe390ba235e42b65bf Author: Michael Spang <spang@chromium.org> Date: Wed Jul 25 20:39:58 2018 Revert "Set correct pixel size for the compositor" This reverts commit c1a826ab79d4303ddd87bbbf4f6d38995c4bcd36. Reason for revert: Screen stops updating on some devices with non default scale factor Bug: 866010 Original change's description: > Set correct pixel size for the compositor > > Right now we clip(GL_SCISSOR) the display buffer based on the physical > pixel size of the display. The pixel size we send to the ui compositor > is the size we receive from the platform window(the display in Chrome > OS). However, this may not be what we want if the platform has some > fractional scale applied. > > UI elements have their sizes set in DIP. If some UI element (ui::Layer) > has its size set to the DIP size of the display, their scaled size > (the pixel size after applying device scale factor) after rounding may > not match the physical pixel size of the display. This difference may > lead to 1px lines left unclipped. This happens in the case of the shelf > whose width is set to the width of the display. For a device like eve > where the internal display has a physical resolution width of 2400px, > if a device scale of 1.8 is applied, the DIP width of the display > becomes 1333. We set the bounds of the shelf to this. Now when we do > the actual paint, we scale the shelf to its physical pixel size which > is ROUND(1333*1.8) = 2399. At the same time the compositor will clip > things at width 2400. This difference is what gives the shelf a 1px > gap. > > A proper way to fix this is to scale ui::Layer bounds such that it > snaps to the parent's or displays edge similar to what pixel canvas > does in views::View. However, that is a large change. In the meantime > this patch sets the correct physical pixel size for the compositor so > that correct clipping happens. > > Bug: 843354 > Change-Id: Ic9e055c0ba39d85a7297809baf946398bf09f40a > Component: UI compositor, pixel canvas, scaling, HiDPI > Reviewed-on: https://chromium-review.googlesource.com/1130495 > Reviewed-by: Mitsuru Oshima <oshima@chromium.org> > Reviewed-by: Nico Weber <thakis@chromium.org> > Commit-Queue: Malay Keshav <malaykeshav@chromium.org> > Cr-Commit-Position: refs/heads/master@{#575402} TBR=oshima@chromium.org,thakis@chromium.org,malaykeshav@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 843354 Change-Id: If6d8e5649f37ec2cece1411419ca2db61daf06c8 Reviewed-on: https://chromium-review.googlesource.com/1150251 Reviewed-by: Michael Spang <spang@chromium.org> Commit-Queue: Michael Spang <spang@chromium.org> Cr-Commit-Position: refs/heads/master@{#578042} [modify] https://crrev.com/48d26c3d8f9f81fe52e33abe390ba235e42b65bf/ash/host/ash_window_tree_host_platform.cc [modify] https://crrev.com/48d26c3d8f9f81fe52e33abe390ba235e42b65bf/ash/host/ash_window_tree_host_platform.h [modify] https://crrev.com/48d26c3d8f9f81fe52e33abe390ba235e42b65bf/ui/aura/window_tree_host.cc [modify] https://crrev.com/48d26c3d8f9f81fe52e33abe390ba235e42b65bf/ui/aura/window_tree_host.h
,
Jul 25
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/01f811977e700cbafa5cd3087b53cea6940dd23f commit 01f811977e700cbafa5cd3087b53cea6940dd23f Author: Michael Spang <spang@chromium.org> Date: Wed Jul 25 20:48:39 2018 Revert "Set correct pixel size for the compositor" This reverts commit c1a826ab79d4303ddd87bbbf4f6d38995c4bcd36. Reason for revert: Screen stops updating on some devices with non default scale factor Bug: 866010 Original change's description: > Set correct pixel size for the compositor > > Right now we clip(GL_SCISSOR) the display buffer based on the physical > pixel size of the display. The pixel size we send to the ui compositor > is the size we receive from the platform window(the display in Chrome > OS). However, this may not be what we want if the platform has some > fractional scale applied. > > UI elements have their sizes set in DIP. If some UI element (ui::Layer) > has its size set to the DIP size of the display, their scaled size > (the pixel size after applying device scale factor) after rounding may > not match the physical pixel size of the display. This difference may > lead to 1px lines left unclipped. This happens in the case of the shelf > whose width is set to the width of the display. For a device like eve > where the internal display has a physical resolution width of 2400px, > if a device scale of 1.8 is applied, the DIP width of the display > becomes 1333. We set the bounds of the shelf to this. Now when we do > the actual paint, we scale the shelf to its physical pixel size which > is ROUND(1333*1.8) = 2399. At the same time the compositor will clip > things at width 2400. This difference is what gives the shelf a 1px > gap. > > A proper way to fix this is to scale ui::Layer bounds such that it > snaps to the parent's or displays edge similar to what pixel canvas > does in views::View. However, that is a large change. In the meantime > this patch sets the correct physical pixel size for the compositor so > that correct clipping happens. > > Bug: 843354 > Change-Id: Ic9e055c0ba39d85a7297809baf946398bf09f40a > Component: UI compositor, pixel canvas, scaling, HiDPI > Reviewed-on: https://chromium-review.googlesource.com/1130495 > Reviewed-by: Mitsuru Oshima <oshima@chromium.org> > Reviewed-by: Nico Weber <thakis@chromium.org> > Commit-Queue: Malay Keshav <malaykeshav@chromium.org> > Cr-Commit-Position: refs/heads/master@{#575402} TBR=malaykeshav@chromium.org, oshima@chromium.org, spang@chromium.org, thakis@chromium.org (cherry picked from commit 48d26c3d8f9f81fe52e33abe390ba235e42b65bf) Bug: 843354 Change-Id: If6d8e5649f37ec2cece1411419ca2db61daf06c8 Reviewed-on: https://chromium-review.googlesource.com/1150251 Reviewed-by: Michael Spang <spang@chromium.org> Commit-Queue: Michael Spang <spang@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#578042} Reviewed-on: https://chromium-review.googlesource.com/1150682 Cr-Commit-Position: refs/branch-heads/3497@{#86} Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753} [modify] https://crrev.com/01f811977e700cbafa5cd3087b53cea6940dd23f/ash/host/ash_window_tree_host_platform.cc [modify] https://crrev.com/01f811977e700cbafa5cd3087b53cea6940dd23f/ash/host/ash_window_tree_host_platform.h [modify] https://crrev.com/01f811977e700cbafa5cd3087b53cea6940dd23f/ui/aura/window_tree_host.cc [modify] https://crrev.com/01f811977e700cbafa5cd3087b53cea6940dd23f/ui/aura/window_tree_host.h
,
Jul 25
,
Jul 31
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fd6fd0c8626d5a5aa02081138c112d06f0633a69 commit fd6fd0c8626d5a5aa02081138c112d06f0633a69 Author: Malay Keshav <malaykeshav@chromium.org> Date: Tue Jul 31 22:07:47 2018 Adjust shelf bounds to cover display edges This patch adjusts the shelf width such that in pixel space there is no gap left between the shelf and the edge of the display. The adjustment is done via a utility method which can be used to adjust any bounds to ensure that they cover the edge of the display or screen. Bug: 843354 Change-Id: Ica9adfe6e21e7ae71ef8206e20507a372317c105 Component: Screen util, shelf Reviewed-on: https://chromium-review.googlesource.com/1154040 Commit-Queue: Malay Keshav <malaykeshav@chromium.org> Reviewed-by: Mitsuru Oshima <oshima@chromium.org> Cr-Commit-Position: refs/heads/master@{#579590} [modify] https://crrev.com/fd6fd0c8626d5a5aa02081138c112d06f0633a69/ash/screen_util.cc [modify] https://crrev.com/fd6fd0c8626d5a5aa02081138c112d06f0633a69/ash/screen_util.h [modify] https://crrev.com/fd6fd0c8626d5a5aa02081138c112d06f0633a69/ash/screen_util_unittest.cc [modify] https://crrev.com/fd6fd0c8626d5a5aa02081138c112d06f0633a69/ash/shelf/shelf_layout_manager.cc
,
Jul 31
,
Aug 30
Issue 878830 has been merged into this issue.
,
Sep 20
,
Oct 8
Issue 842198 has been merged into this issue.
,
Oct 8
Issue 854454 has been merged into this issue. |
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 by malaykeshav@chromium.org
, May 15 2018