[LayoutNG] 'overflow: auto' is broken when 'background' is set and on hi-dpi display |
||||||||||||
Issue descriptionChrome Version: 69.0.3485.0 with chrome://flags/#enable-layout-ng enabled OS: macOS 10.13 What steps will reproduce the problem? (1) Open https://ja.stackoverflow.com/questions/45438/%E3%81%93%E3%81%AE%E3%82%B3%E3%83%BC%E3%83%89%E3%81%AE%E9%87%8F%E3%82%92%E6%B8%9B%E3%82%89%E3%81%97%E3%81%9F%E3%81%84 What is the expected result? the page is rendered same as non-layoutng. What happens instead? A <pre> block is broken. - Background-color of the PRE is not painted on whole lines - Hooter part, [javascript] Share Edit Close..., is duplicated. Please use labels and text to provide additional information.
,
Jul 18
Still reproducible with 69.0.3494.0 canary on macOS Retina.
,
Jul 18
Not reproducible on Windows.
,
Jul 23
No longer reproduces on linux either. Marking as fixed.
,
Jul 24
Still reproducible with 70.3500.0 canary + macOS 10.3 *Retina*. Not reproducible with non-Retina setting.
,
Jul 30
Ah, thanks tkent!
,
Jul 31
English page example: https://stackoverflow.com/questions/51605578/angularjs-http-cors-and-basic-authentication-with-jersey PRE + scroll + probably some CSS properties used in stackoverflow.com, havenY't narrowed down what the property is yet.
,
Aug 30
Emil, I suspect this happens because NG computes VisualRect, overflow rect, or one of these paint-related rects, but hard to diagnose further for me. Happen to know anyone who can advise which rects are likely to be wrong?
,
Aug 30
Hmm, interesting. Any ideas Christian?
,
Aug 30
Hm... we use legacy to compute the overflow rect (we just call ComputeOverflow), so presumably that one is correct? So I guess maybe it's the visual rect?
,
Aug 30
Aleks points out that this is not true (we do override AddOverflowFromChildren in the mixin)
,
Sep 4
I found I can reproduce this on my Huawei P20 Pro, screen shot attached. Also on another URL: https://drafts.css-houdini.org/font-metrics-api-1/
,
Sep 4
,
Sep 4
,
Sep 4
Reproduces on local chrome but not on content_shell. VisualRect and scroll_width/_height look the same as legacy. Whatelse could cause symptom like this?
,
Sep 7
Found how code path is different, need to investigate why. Here's a memo. In NGBoxFragmentPainter::PaintBoxDecorationBackground, when running chrome.exe on 150% display v.s. 100% display: * IsPaintingBackgroundOfPaintContainerIntoScrollingContentsLayer returns true on 150% while false on 100%, so paint_rect is empty on 150%. * Then paint_info.SkipRootBackground() is true on 150%, and that should_paint_background becomes false. Because should_paint_background is false on 150% display, NGBoxFragmentPainter doesn't call PaintBackground.
,
Sep 9
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1160492e162ce25d33ba67bf738f6e28e33d8b20 commit 1160492e162ce25d33ba67bf738f6e28e33d8b20 Author: Koji Ishii <kojii@chromium.org> Date: Sun Sep 09 21:09:35 2018 [LayoutNG] Fix NGBoxFragmentPainter to paint background of scrolling contents layer The code to paint the background of scrolling contents layer was missing in NGBoxFragmentPainter. This patch copies the logic from BoxPainter. Issue 861623 found that this code path runs when all of the following conditions are true: * In Chrome (not in content_shell). * When display / CSS pixel >= 1.5. * When background is set to 'overflow: auto' box. This patch also fixes 3 CSS contain tests that has 'contain: paint', 'overflow-[xy]: scroll', and background color to an inline formatting context. Bug: 861623 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng;luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel Change-Id: I31d0b407e794409552dcf89f5c8005fafc5f99a6 Reviewed-on: https://chromium-review.googlesource.com/1215143 Reviewed-by: Emil A Eklund <eae@chromium.org> Commit-Queue: Emil A Eklund <eae@chromium.org> Cr-Commit-Position: refs/heads/master@{#589809} [modify] https://crrev.com/1160492e162ce25d33ba67bf738f6e28e33d8b20/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=LayoutNG [modify] https://crrev.com/1160492e162ce25d33ba67bf738f6e28e33d8b20/third_party/blink/renderer/core/paint/ng/ng_box_fragment_painter.cc
,
Sep 10
,
Sep 12
Canary got much better, but still has some issues. Will sync more recent paint changes to NG.
,
Sep 12
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/07172170d6f94704bd47389e39251e24b905baaa commit 07172170d6f94704bd47389e39251e24b905baaa Author: Koji Ishii <kojii@chromium.org> Date: Wed Sep 12 17:30:23 2018 [LayoutNG] Sync PaintBoxDecorationBackground with BoxPainter Following r589809 (CL:1215143), this patch synchronizes NGBoxFragmentPainter::PaintBoxDecorationBackground with the current BoxPainter. These functions in BoxPainter has had several changes. Also, features not implemented before such as theme painting are implemented in this patch. Not much efforts to eliminate LayoutObject dependency were done in this patch yet. Bug: 861623 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng;luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel Change-Id: Ia965fc0f466b15517702b78096d40e9788219cf3 Reviewed-on: https://chromium-review.googlesource.com/1218187 Commit-Queue: Emil A Eklund <eae@chromium.org> Reviewed-by: Emil A Eklund <eae@chromium.org> Cr-Commit-Position: refs/heads/master@{#590740} [modify] https://crrev.com/07172170d6f94704bd47389e39251e24b905baaa/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=LayoutNG [modify] https://crrev.com/07172170d6f94704bd47389e39251e24b905baaa/third_party/WebKit/LayoutTests/flag-specific/enable-blink-features=LayoutNG/paint/invalidation/svg/add-background-property-on-root-expected.txt [modify] https://crrev.com/07172170d6f94704bd47389e39251e24b905baaa/third_party/WebKit/LayoutTests/flag-specific/enable-blink-features=LayoutNG/paint/invalidation/svg/remove-background-property-on-root-expected.txt [modify] https://crrev.com/07172170d6f94704bd47389e39251e24b905baaa/third_party/blink/renderer/core/paint/ng/ng_box_fragment_painter.cc [modify] https://crrev.com/07172170d6f94704bd47389e39251e24b905baaa/third_party/blink/renderer/core/paint/ng/ng_box_fragment_painter.h
,
Sep 13
https://ja.stackoverflow.com/questions/45438/%E3%81%93%E3%81%AE%E3%82%B3%E3%83%BC%E3%83%89%E3%81%AE%E9%87%8F%E3%82%92%E6%B8%9B%E3%82%89%E3%81%97%E3%81%9F%E3%81%84 https://stackoverflow.com/questions/51605578/angularjs-http-cors-and-basic-authentication-with-jersey https://drafts.css-houdini.org/font-metrics-api-1/ All pages look good on Mac Retina and on Win (150%). |
||||||||||||
►
Sign in to add a comment |
||||||||||||
Comment 1 by e...@chromium.org
, Jul 18Summary: [LayoutNG] PRE block in stackoverlfow.com is broken (was: NG: PRE block in stackoverlfow.com is broken)