Bug 110762 – [Texmap] TextureMapper is too eager to use intermediate surfaces.

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Noam Rosenthal]

[Texmap] TextureMapper is too eager to use intermediate surfaces.

[Noam Rosenthal]

Created attachment 190071 [details]
Patch

[EFL EWS Bot]

Comment on attachment 190071 [details]
Patch

Attachment 190071 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/16746404

[Early Warning System Bot]

Comment on attachment 190071 [details]
Patch

Attachment 190071 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/16743402

[WebKit Review Bot]

Comment on attachment 190071 [details]
Patch

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

New failing tests:
platform/chromium/virtual/softwarecompositing/overlap-blending/children-opacity-huge.html
compositing/overlap-blending/children-opacity-no-overlap.html
platform/chromium/virtual/softwarecompositing/overlap-blending/reflection-opacity-huge.html
compositing/overlap-blending/reflection-opacity-huge.html
compositing/overlap-blending/children-opacity-huge.html

[Build Bot]

Comment on attachment 190071 [details]
Patch

Attachment 190071 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/16750430

New failing tests:
compositing/overlap-blending/children-opacity-huge.html
compositing/overlap-blending/reflection-opacity-huge.html

[Build Bot]

Comment on attachment 190071 [details]
Patch

Attachment 190071 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://queues.webkit.org/results/16746467

New failing tests:
compositing/overlap-blending/reflection-opacity-huge.html
compositing/overlap-blending/children-opacity-huge.html

[Noam Rosenthal]

Created attachment 190230 [details]
Patch

[EFL EWS Bot]

Comment on attachment 190230 [details]
Patch

Attachment 190230 [details] did not pass efl-ews (efl):
Output: http://webkit-commit-queue.appspot.com/results/16759777

[Noam Rosenthal]

Created attachment 190236 [details]
Patch

[Noam Rosenthal]

Created attachment 190244 [details]
Patch

[Build Bot]

Comment on attachment 190244 [details]
Patch

Attachment 190244 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-commit-queue.appspot.com/results/16760782

New failing tests:
compositing/overlap-blending/reflection-opacity-huge.html

[Build Bot]

Comment on attachment 190244 [details]
Patch

Attachment 190244 [details] did not pass mac-ews (mac):
Output: http://webkit-commit-queue.appspot.com/results/16772020

New failing tests:
compositing/overlap-blending/reflection-opacity-huge.html

[Noam Rosenthal]

Created attachment 190262 [details]
Patch

[Kenneth Rohde Christiansen]

Comment on attachment 190262 [details]
Patch

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

> Source/WebCore/ChangeLog:17
> +        and "non overlapping" regions. The non-overlapping regions are painted directly, while the overlapping
> +        regions are tiled to smaller rectangles painted using an intermediate surface.

These are overlapping, transparent regions?

[Noam Rosenthal]

(In reply to comment #14)
> (From update of attachment 190262 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=190262&action=review
> 
> > Source/WebCore/ChangeLog:17
> > +        and "non overlapping" regions. The non-overlapping regions are painted directly, while the overlapping
> > +        regions are tiled to smaller rectangles painted using an intermediate surface.
> 
> These are overlapping, transparent regions?

Overlapping regions that need to be blended to a surface before opacity/masks/filters are applied.
E.g. a parent with opacity and two overlapping children that are not necessarily transparent. See LayoutTests/reflections/reflection-opacity.html for example.

[Kenneth Rohde Christiansen]

Comment on attachment 190262 [details]
Patch

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

> Source/WebCore/ChangeLog:3
> +        [Texmap] TextureMapper is too eager to use intermediate surfaces.

No dot please.

> Source/WebCore/ChangeLog:12
> +        target surface with the layer's opacity and mask.
> +        This would make it so that (1) surfaces are created even when they're not needed, i.e. when there

Add a newline before this (good description though!)

> Source/WebCore/ChangeLog:74
> +        (CoordinatedBackingStore):
> +                Removed the "mask" parameter from TextureMapperPlatformLayer and overrides.

a why would be nice (or refer to the description below)

> Source/WebCore/platform/graphics/texmap/TextureMapper.h:186
>      TextDrawingModeFlags m_textDrawingMode;
>      OwnPtr<BitmapTexturePool> m_texturePool;
>      AccelerationMode m_accelerationMode;
> +    bool m_maskMode;

maybe it should be called m_isMaskMode as all the other m_ ...Mode are enums

> Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp:254
> -void TextureMapperLayer::paintRecursive(const TextureMapperPaintOptions& options)
> +static void resolveOverlaps(Region& overlapRegion, Region& nonOverlapRegion, Region newRegion)
>  {

wouldnt it make sense to have output values after input? maybe s/newRegion/sourceRegion/

> Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp:317
> +    if (!alwaysResolveSelfOverlap  && shouldBlend()) {

two spaces before && :-)

[Noam Rosenthal]

Created attachment 190285 [details]
Patch

[Allan Sandfeld Jensen]

Comment on attachment 190285 [details]
Patch

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

> Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp:346
> +        const static int maxRectDimension = 2000;

Could this be moved to a global constant, or tied to the texture dimensions defined for GL and ImageBuffer TextureMapper?

[Noam Rosenthal]

Created attachment 190343 [details]
Patch

[WebKit Review Bot]

Comment on attachment 190343 [details]
Patch

Attachment 190343 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://webkit-commit-queue.appspot.com/results/16784004

New failing tests:
compositing/overlap-blending/children-opacity-no-overlap.html
platform/chromium/virtual/softwarecompositing/overlap-blending/reflection-opacity-huge.html
compositing/overlap-blending/children-opacity-huge.html

[Noam Rosenthal]

Created attachment 190453 [details]
Patch

[WebKit Review Bot]

Comment on attachment 190453 [details]
Patch

Attachment 190453 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://webkit-commit-queue.appspot.com/results/16823048

New failing tests:
platform/chromium/virtual/softwarecompositing/overlap-blending/reflection-opacity-huge.html

[Noam Rosenthal]

Created attachment 190467 [details]
Patch

[Allan Sandfeld Jensen]

Comment on attachment 190467 [details]
Patch

r=me

[WebKit Review Bot]

Comment on attachment 190467 [details]
Patch

Clearing flags on attachment: 190467

Committed r144190: <http://trac.webkit.org/changeset/144190>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.