Bug 77557 - Introduce a flag to denote layers which should follow the page scale
Summary: Introduce a flag to denote layers which should follow the page scale
Status: UNCONFIRMED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 68075
  Show dependency treegraph
 
Reported: 2012-02-01 09:07 PST by Sami Kyostila
Modified: 2012-02-10 12:22 PST (History)
5 users (show)

See Also:


Attachments
Patch (6.34 KB, patch)
2012-02-01 09:14 PST, Sami Kyostila
no flags Details | Formatted Diff | Diff
Patch (16.19 KB, patch)
2012-02-10 11:32 PST, Sami Kyostila
simon.fraser: review-
simon.fraser: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sami Kyostila 2012-02-01 09:07:39 PST
The contents scale factor of GraphicsLayerChromium is set according to the device and page scale in deviceOrPageScaleFactorChanged(). The problem is that if the page scale factor never changes, the contents scale never gets set to the page scale factor that was in effect at the time of the layer's creation. If the page scale scale was != 1, this leads to incorrect contentBounds() on the associated LayerChromium.
Comment 1 Sami Kyostila 2012-02-01 09:14:34 PST
Created attachment 124958 [details]
Patch
Comment 2 WebKit Review Bot 2012-02-01 09:47:47 PST
Comment on attachment 124958 [details]
Patch

Attachment 124958 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11387524

New failing tests:
compositing/scaling/tiled-layer-recursion.html
Comment 3 Sami Kyostila 2012-02-02 11:30:13 PST
FYI, the failure from chromium-ews seems like a flake since I have not been able to reproduce it locally. I'll try again tomorrow though.
Comment 4 James Robinson 2012-02-02 16:13:02 PST
Comment on attachment 124958 [details]
Patch

R=me, let's let the commit queue take a crack at that patch to see if it helps.
Comment 5 WebKit Review Bot 2012-02-02 17:24:10 PST
Comment on attachment 124958 [details]
Patch

Rejecting attachment 124958 [details] from commit-queue.

New failing tests:
compositing/scaling/tiled-layer-recursion.html
Full output: http://queues.webkit.org/results/11394983
Comment 6 Sami Kyostila 2012-02-10 10:14:47 PST
Ok, I figured out what was up with the failing test. The page scale is affecting the scroll bars and they are getting scaled after being drawn, causing some filtering artifacts. First I thought this caused by some GTK theme weirdness on my machine but that was a red herring.

Currently the overflow control layers (scroll bars) are configured appliesPageScale=false. This is a signal to GraphicsLayerChromium to configure the underlying LayerChromium with a content scale to implement the actual page scale.

However, since the LayerChromium content scale is only updated when 1) the layer transform changes or 2) the page scale for the layer or a parent of it changes, the content scale never ends up getting set for the overflow controls layer and the scroll bars look as they should.

My change was to always apply the content scale for GraphicsLayerChromium, and this broke the scroll bars since they were now scaled.

I've now revised the patch to set appliesPageScale=true for overflow controls and made sure changing that flag is properly reflected in LayerChromium and onwards.
Comment 7 Sami Kyostila 2012-02-10 11:32:19 PST
Created attachment 126544 [details]
Patch
Comment 8 Sami Kyostila 2012-02-10 11:34:59 PST
Changed title since this the changes are not limited to Chromium any more. Let me know if you think I should split the patch into two pieces.
Comment 9 Simon Fraser (smfr) 2012-02-10 12:22:47 PST
Comment on attachment 126544 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=126544&action=review

r- because of the added ambiguity.

> Source/WebCore/ChangeLog:9
> +          1. Layer contents are scaled by the painter (e.g., non-composited
> +             content layer)

"the painter" is not a term we use in WebKit. I don't really know what you mean by it.

> Source/WebCore/ChangeLog:37
> +        Reviewed by NOBODY (OOPS!).

This should go above the description.

> Source/WebCore/platform/graphics/GraphicsLayer.cpp:86
> +    , m_needsPageScale(true)

Currently layers which are descendants of the layer with m_appliesPageScale=true get scaled. Put another way, the layer with m_appliesPageScale=true is the root of the scale.

Now that you've added m_needsPageScale, there is ambiguity. What does it mean for a layer which is ancestral to the m_appliesPageScale=true layer to have m_needsPageScale=true?

Maybe we should pull out m_appliesPageScale entirely, and have RLC store which layer is the scale root. Instead of m_needsPageScale, you could then have m_ignoresPageScale, which is set to true for overflow control layers.