Issue metadata
Sign in to add a comment
|
CSS drop-shadow and transform: scale3d are interfering with each other
Reported by
nerdki...@gmail.com,
Apr 20 2018
|
|||||||||||||||||||||||||
Issue descriptionUserAgent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/67.0.3396.10 Safari/537.36 Steps to reproduce the problem: 1. Create a div (or other block element) that has a different background color than its parent element 2. Apply a drop-shadow rule (like in Material Design specs) 3. Apply a transform scale3d(1, 1, 1) rule 4. Note the difference in color, the drop shadow is lightening the background color instead of applying shadows in addition to the background color of the parent element. (I made a jsfiddle with the bug: https://jsfiddle.net/ucqhsmqz) What is the expected behavior? Do the same thing in Chrome stable and note that the drop-shadow behaves as expected, the drop shadow doesn't lighten the background-color of the parent element. Firefox Developer Edition also handles this case correctly What went wrong? Chrome is rendering the drop-shadow incorrectly by seemingly assuming that the background color of the parent element is white. Did this work before? Yes 66.0.3359.117 Does this work in other browsers? Yes Chrome version: 67.0.3396.10 Channel: dev OS Version: Ubuntu 17.10 64-bit Flash Version:
,
Apr 23 2018
Unable to reproduce this issue on reported version 67.0.3396.10 using Windows 10, Ubuntu 17.10 and Mac 10.13.3 with JSFiddle given in comment#0. Attaching screenshots for reference. @Reporter: Please check the screenshots and let us know if we miss anything. Also please check the issue on fresh profile which do not have any apps/extensions. Any further info on reproducing the issue would help in further debugging. Thanks!
,
Apr 23 2018
I tried opening the JSFiddle again and I was still seeing the same issues. Thinking it might be something to do with extensions or Chrome flags, I reset all of my flags to the default values, and I uninstalled all of my extensions. I still saw the same issue. However, I noticed that dragging the Chrome window from my external monitor to my laptop monitor, the drop shadow issue went away. Dragging it back to my external monitor made the issue come back. My first guess would be that this is no longer a Chrome issue but a Ubuntu issue, but this monitor switching bug with drop shadows still doesn't happen in Chrome 66 (stable) when I tested it on my same machine. I will generate some screenshots and attach them below, but I know that it will be difficult to tell what's happening based off screenshots alone.
,
Apr 23 2018
Thank you for providing more feedback. Adding the requester to the cc list. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Apr 23 2018
Here are the screenshots I mentioned earlier. I tested this in Chrome 67 (dev) and Chrome 66 (stable) on Ubuntu 17.10 installed on Dell Precision laptop with external Dell monitor using Nvidia binary graphics driver v384.111
,
Apr 23 2018
Here are the screenshots of this not being an issue in Chrome 66 (stable). (I ran into the upload size limit in the last comment.)
,
Apr 23 2018
,
Apr 24 2018
Able to reproduce this issue on 67.0.3396.10 and latest dev 68.0.3404.0 using dual monitor setup on Ubuntu 17.10. Issue is not seen in Windows. We so not have Mac dual monitor setup as of now to check this there. Good Build: 67.0.3387.0 Bad Build: 67.0.3388.0 Unable to provide bisect info as all builds invoked are good, tried by increasing the range but got same changelog. Hence assigning from manual changelog. CL: https://chromium.googlesource.com/chromium/src/+log/67.0.3387.0..67.0.3388.0?pretty=fuller&n=10000 Suspecting https://chromium.googlesource.com/chromium/src/+/8f035f982a922db68ce76a374453dc1508b7bdf4 from changelog. @fs: Please confirm the bug and help in re-assigning if this is not related to your change. Adding RB-Stable as this is a recent regression. Please remove if not the case. Thanks!
,
Apr 24 2018
Definitely not that CL... I can't really confirm the issue, but based on the data provided it would seem to be a colorspace issue. So the change in behavior might stem from 6b2841f34cb726c6472a72604481215829266d6e, although it's not clear whether it is a bug or not. Reporter, could you check if you have any color profile setup for the external display, and if so perhaps attach a copy?
,
Apr 24 2018
This seems to happen when we render in a different color space than we raster in. Something that wasn't possible on ubuntu before my change, as we would read one ICC profile and apply it equally to all monitors. Now, we read all ICC profiles, but we use the "primary" one to decide the rastering colorspace. The actual bug is likely to be old and perhaps difficult to fix. Maybe ccameron has more information.
,
Apr 24 2018
I think the bisect is suspect -- if we can consistently reproduce this, we can get a more narrow bisect.
,
Apr 24 2018
@fs I have the `Automatic - Precision 5520` color profile for my internal display, and `Automatic - DELL U2415` profiles for my external displays. I did some searching to find the *.icc files on my machine for those profiles and I generated the following two files using colord's colormgr. Let me know if these are what you're looking for. NOTE: I obfuscated some of the data in these files to protect my userdata.
,
Apr 24 2018
So I can reproduce something similar on my machine. I have one wide-gamut monitor and one sRGB monitor. On the sRGB monitor, there is no shadow. On the wide-gamut monitor there is a shadow, but it is darker than the background, not brighter like the initial report.
,
Apr 24 2018
Btw, this bug also makes me think that when the user has multiple monitors, we should raster in the widest available color space, not the color space of the primary monitor.
,
Apr 24 2018
Re #14 -- we re-raster as soon as the window moves to a different display ... so this should be identical to the single-monitor situation. If you go to about:flags and set "Force Color Profile" to "Display P3 D65", does the bug reproduce?
,
Apr 24 2018
No, when I force the color profile it does not reproduce.
,
Apr 25 2018
M67 Stable promotion is coming soon. Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and request a merge into the release branch ASAP. Thank you.
,
Apr 25 2018
I think this issue might be related to Issue 820233. The screenshot from Nvidia's site looks remarkably similar to the jsfiddle I created to demo my own issue.
,
Apr 25 2018
nerdkid93, could yo attach your ICC profiles? I'm not sure why your get-devices.txt lists the same monitor twice, and then lists 2 or 3 ICC profiles for each of those though. Maybe /var/lib/gdm3/.local/share/icc/edid-4b37f4944a6c9b8fd13eb57c156130ff.icc is the most relevant one? (since it's the first one...)
,
Apr 27 2018
It lists the same monitor twice because I have two of them! I turned one off during the screenshots to cut down on the horizontal width of the images. Attached is the requested monitor color profile.
,
Apr 27 2018
Did some additional testing, and it seems like the problem is caused by having different transfer functions on different monitors. Your ICC profile is a gamma=2.2 profile, which is similar, but not identical to the standard sRGB transfer function. If I assign this ICC profile to my primary monitor and leave the secondary monitor unset (=sRGB) I get the same brightening that you do. If I assign the same ICC profile to both monitors I do not. I tried some other ICC profiles, and those with different transfer functions seem to cause these sort of problems. Chris, does this make any sense to you?
,
Apr 30 2018
This is very confusing to me. I wonder what happens if we change LayerTreeHostImpl::GetRasterColorSpace to always return one color space, and use --force-display-color-profile to another.
,
Apr 30 2018
I hacked up LayerTreeHostImpl::GetRasterColorSpace to return ... ... sRGB: no visible border around the square (which I think is correct) ... sRGB, but with gamma 2.8: dark border around the square ... sRGB, but with gamma 1.8: light border around the square I'm guessing that the border pixels "forget" their color space somewhere. Note, I didn't use --force-color-profile, but the display I'm using right now is sRGB, so it shouldn't matter.
,
Apr 30 2018
Might be worth checking that the color in the drop-shadow(...) (#d5d5d5) gets adapted properly. (I think that would be via GLRenderer::UpdateRPDQWithSkiaFiltersor so.)
,
May 1 2018
I think I figured this out. The problem is that the color management code basically does: color.rgb = Transform(color.rgb); However, the colors are pre-multiplied alpha, so the alpha data gets messed up. The solution is basically to do: color.rgb = Transfor(color.rgb/color.a) * color.a Potential fix CL is on the way.
,
May 1 2018
,
May 1 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ed480969eb31afdfdad0cf3d42206b3a3314077e commit ed480969eb31afdfdad0cf3d42206b3a3314077e Author: Fredrik Hubinette <hubbe@google.com> Date: Tue May 01 23:31:23 2018 Color: handle pre-multiplied alpha correctly Without this change, we're doing color transforms on the multiplied-in alpha values, which is wrong. Bug: 835322 Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel Change-Id: I30a9533837f8d6392394eab9d859968cec93e4da Reviewed-on: https://chromium-review.googlesource.com/1036618 Reviewed-by: Fredrik Hubinette <hubbe@chromium.org> Reviewed-by: ccameron <ccameron@chromium.org> Commit-Queue: Fredrik Hubinette <hubbe@chromium.org> Cr-Commit-Position: refs/heads/master@{#555226} [modify] https://crrev.com/ed480969eb31afdfdad0cf3d42206b3a3314077e/components/viz/service/display/renderer_pixeltest.cc [modify] https://crrev.com/ed480969eb31afdfdad0cf3d42206b3a3314077e/components/viz/service/display/shader.cc [modify] https://crrev.com/ed480969eb31afdfdad0cf3d42206b3a3314077e/third_party/WebKit/LayoutTests/platform/linux/hdr/video-canvas-alpha-expected.png [modify] https://crrev.com/ed480969eb31afdfdad0cf3d42206b3a3314077e/third_party/WebKit/LayoutTests/platform/mac/hdr/video-canvas-alpha-expected.png [modify] https://crrev.com/ed480969eb31afdfdad0cf3d42206b3a3314077e/third_party/WebKit/LayoutTests/platform/win/hdr/video-canvas-alpha-expected.png
,
May 2 2018
,
May 2 2018
[Auto-generated comment by a script] We noticed that this issue is targeted for M-67; it appears the fix may have landed after branch point, meaning a merge might be required. Please confirm if a merge is required here - if so add Merge-Request-67 label, otherwise remove Merge-TBD label. Thanks.
,
May 2 2018
,
May 2 2018
CL listed at #28 missed today' canary cut by few cls. Pls update the bug with canary result on Thursday morning. Thank you. Removing "Merge-TBD" label as M67 merge is already requested at #31.
,
May 2 2018
If I understand this CL correctly, it seems that drop-shadows configured with CSS are actually handled using the WebGL/OpenGL shader language. Is that correct? Or was this CSS issue also causing issues in WebGL/OpenGL?
,
May 2 2018
Drop-shadows are drawn by Skia (which may use OpenGL to do it, or not.) The drawn drop-shadows use the alpha channel to draw the shadows. (semitransparent) The bug here is that in some cases, the drop-shadows are drawn in a different color space than the screen color space, and we were doing the conversion in a way which also affected the alpha channel, which wasn't supposed to happen.
,
May 2 2018
Thanks for the explanation! I have been trying to get a feel for how Chromium works for a couple months, but the source code is more than a little intimidating for me (someone who knows only a little c++), so it was nice to be able to understand how this change fixes this issue.
,
May 2 2018
This bug requires manual review: M67 has already been promoted to the beta branch, so this requires manual review Please contact the milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
May 3 2018
The NextAction date has arrived: 2018-05-03
,
May 3 2018
Able to reproduce this issue on reported version using Ubuntu 17.10. Hence verifying the fix on latest canary 68.0.3418.0. Now not observing any CSS drop-shadow on opening jsfiddle in secondary monitor. Attaching screenshot for reference. As fix is working as expected adding TE-Verified labels. Thanks!
,
May 3 2018
hubbe@, Cl is already verified by Test team at #38 in canary. How is the change looking to you in canary? Is it safe to merge now?
,
May 3 2018
As far as I can tell, there is no new crashes or bugreports associated with this change, so I think it is safe to merge.
,
May 3 2018
Approving merge to M67 branch 3396 based on comment #38 and #40. Please merge ASAP. Thank you.
,
May 3 2018
Had to resolve some minor merge conflicts, hopefully I didn't break anything.
,
May 3 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5feeb618b6f8859d66195ea223c469732f7d5d9c commit 5feeb618b6f8859d66195ea223c469732f7d5d9c Author: Fredrik Hubinette <hubbe@google.com> Date: Thu May 03 18:07:39 2018 Color: handle pre-multiplied alpha correctly Without this change, we're doing color transforms on the multiplied-in alpha values, which is wrong. TBR=hubbe@google.com (cherry picked from commit ed480969eb31afdfdad0cf3d42206b3a3314077e) Bug: 835322 Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel Change-Id: I30a9533837f8d6392394eab9d859968cec93e4da Reviewed-on: https://chromium-review.googlesource.com/1036618 Reviewed-by: Fredrik Hubinette <hubbe@chromium.org> Reviewed-by: ccameron <ccameron@chromium.org> Commit-Queue: Fredrik Hubinette <hubbe@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#555226} Reviewed-on: https://chromium-review.googlesource.com/1042810 Cr-Commit-Position: refs/branch-heads/3396@{#460} Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428} [modify] https://crrev.com/5feeb618b6f8859d66195ea223c469732f7d5d9c/components/viz/service/display/renderer_pixeltest.cc [modify] https://crrev.com/5feeb618b6f8859d66195ea223c469732f7d5d9c/components/viz/service/display/shader.cc [modify] https://crrev.com/5feeb618b6f8859d66195ea223c469732f7d5d9c/third_party/WebKit/LayoutTests/platform/linux/hdr/video-canvas-alpha-expected.png [modify] https://crrev.com/5feeb618b6f8859d66195ea223c469732f7d5d9c/third_party/WebKit/LayoutTests/platform/mac/hdr/video-canvas-alpha-expected.png [modify] https://crrev.com/5feeb618b6f8859d66195ea223c469732f7d5d9c/third_party/WebKit/LayoutTests/platform/win/hdr/video-canvas-alpha-expected.png
,
May 7 2018
Issue has been fixed in Chrome Dev (v 68.0.3418.2) Thanks! |
||||||||||||||||||||||||||
►
Sign in to add a comment |
||||||||||||||||||||||||||
Comment 1 by susan.boorgula@chromium.org
, Apr 22 2018