Closed Bug 1181554 Opened 5 years ago Closed 5 years ago
DPI] box-shadow rendered unevenly at uneven scaling factors, e .g . 150%, 250%
STR: - change your OS scaling factor to 150% - log out or restart if your OS suggests doing so (logging out should be sufficient on Windows) - log in again, start Firefox, load the reduced testcase that phlsa is going to attach Actual result: the box-shadow is one device pixel thick on some sides, two devices pixels on others Expected result: As with e.g. CSS borders, the box-shadow should have the same thickness on all four sides We've seen this on Windows 10, need to confirm our assumption that this happens on older versions and other OSes too. dbaron, can you or one of your peers confirm that this is a layout bug?
Here's a very simple test case that illustrates the issue. I was also able to trigger it using Firefoxes built-in text scaling.
I can't seem to reproduce this on Ubuntu...
Summary: [HiDPI] box-shadow rendered unevenly at uneven scaling factors e.g. (150%, 250%) → [HiDPI] box-shadow rendered unevenly at uneven scaling factors, e.g. 150%, 250%
(In reply to Dão Gottwald [:dao] from comment #2) > I can't seem to reproduce this on Ubuntu... That is by changing DPI settings. I *can* reproduce on Ubuntu by using full zoom in Firefox, i.e. Ctrl-+.
OS: Windows 10 → All
I see it on Linux after pressing Ctrl-+ twice. I agree it would make sense to snap spread to device pixels. It's not clear to me whether we'd need to do anything for blur to avoid the same problem there, but I suspect not. It's also not clear that it would need to be done as early in the process as we do it for border, although I don't think it would be harmful.
Is there a chance that we can get this prioritized and fixed soon? Sounds as if it might be an easy fix? This is affecting Firefox theme changes which are riding trains and are even expected to be uplifted to beta.
Can you test this patch? I've only fixed up inset box shadows here and it seems to fix the testcase for me.
Attachment #8631337 - Flags: feedback?
Attachment #8631337 - Flags: feedback? → feedback?(dao)
Comment on attachment 8631337 [details] [diff] [review] patch for inset box shadows This fixes it for me. Thanks! I've noticed that when zooming in repeatedly, the border and the box-shadow reach a thickness of 2 device pixels at the same zoom level, as it should be. This patch only takes care of inset box-shadows, outside ones should probably get the same treatment.
Attachment #8631337 - Flags: feedback?(dao) → feedback+
Assignee: nobody → mstange
Status: NEW → ASSIGNED
Comment on attachment 8631337 [details] [diff] [review] patch for inset box shadows Jeff reviewed this in person and asked me to move all the snapping code closer together in the function. I'll do that and re-request review on the updated patch.
Is there a risk that when this is rounding to larger numbers, overflow areas or display item bounds or IsInvisibleInRect might be off, and we end up with glitches after dynamic changes?
Any updates here? This is pretty noticeable in primary UI for Windows 10, and we'll want to either uplift this fix to Firefox 40 or find a workaround.
I'm sorry, I completely forgot about this! I'll deal with it first thing tomorrow morning.
This moves a few variables so that most of the rounding code is in the same place. It also adds a comment and removes a misleading call to NS_lround.
Attachment #8635458 - Flags: review?(jmuizelaar) → review+
(In reply to David Baron [:dbaron] ⏰UTC+2 (busy until July 20) from comment #10) > Is there a risk that when this is rounding to larger numbers, overflow areas > or display item bounds or IsInvisibleInRect might be off, and we end up with > glitches after dynamic changes? I'm only changing inset box shadows here. The overflow areas / display item bounds aren't affected by an increased spread radius because we're only growing towards the inside of the rect, not towards the outside. The outside is still determined by the padding rect which clips the shadow. If nsDisplayBoxShadowInner had an implementation of IsInvisibleInRect, we'd have to tweak that, but it doesn't.
Woot, thanks! Can you do the aurora+beta uplift request too?
Comment on attachment 8635458 [details] [diff] [review] patch for inset box shadows Sure! Approval Request Comment [Feature/regressing bug #]: needed for upcoming Firefox theme changes [User impact if declined]: uneven borders in the Firefox UI [Describe test coverage new/current, TreeHerder]: none [Risks and why]: extremely low, small fix in isolated parts of the code [String/UUID change made/needed]: none
Verified fixed on latest Nightly, build ID: 20150721030212. Tested on a MS Pro 2 device running Windows 10 64-bit.
Comment on attachment 8635458 [details] [diff] [review] patch for inset box shadows Verified fix required for theme changes. Beta+ Aurora+
Also verified with: - latest Aurora, build ID: 20150723004007; - Firefox 40 beta 7, build ID: 20150720220238.
You need to log in before you can comment on or make changes to this bug.