Bug 110762 - [Texmap] TextureMapper is too eager to use intermediate surfaces.
Summary: [Texmap] TextureMapper is too eager to use intermediate surfaces.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Noam Rosenthal
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-02-25 08:42 PST by Noam Rosenthal
Modified: 2013-02-27 08:18 PST (History)
18 users (show)

See Also:


Attachments
Patch (79.51 KB, patch)
2013-02-25 09:09 PST, Noam Rosenthal
no flags Details | Formatted Diff | Diff
Patch (80.13 KB, patch)
2013-02-26 00:54 PST, Noam Rosenthal
no flags Details | Formatted Diff | Diff
Patch (80.68 KB, patch)
2013-02-26 01:14 PST, Noam Rosenthal
no flags Details | Formatted Diff | Diff
Patch (81.35 KB, patch)
2013-02-26 02:08 PST, Noam Rosenthal
no flags Details | Formatted Diff | Diff
Patch (82.95 KB, patch)
2013-02-26 04:57 PST, Noam Rosenthal
no flags Details | Formatted Diff | Diff
Patch (83.04 KB, patch)
2013-02-26 08:02 PST, Noam Rosenthal
no flags Details | Formatted Diff | Diff
Patch (83.06 KB, patch)
2013-02-26 12:51 PST, Noam Rosenthal
no flags Details | Formatted Diff | Diff
Patch (83.03 KB, patch)
2013-02-26 23:41 PST, Noam Rosenthal
no flags Details | Formatted Diff | Diff
Patch (83.12 KB, patch)
2013-02-27 01:45 PST, Noam Rosenthal
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Noam Rosenthal 2013-02-25 08:42:21 PST
[Texmap] TextureMapper is too eager to use intermediate surfaces.
Comment 1 Noam Rosenthal 2013-02-25 09:09:58 PST
Created attachment 190071 [details]
Patch
Comment 2 EFL EWS Bot 2013-02-25 09:15:36 PST
Comment on attachment 190071 [details]
Patch

Attachment 190071 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/16746404
Comment 3 Early Warning System Bot 2013-02-25 09:17:05 PST
Comment on attachment 190071 [details]
Patch

Attachment 190071 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/16743402
Comment 4 WebKit Review Bot 2013-02-25 09:57:41 PST
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
Comment 5 Build Bot 2013-02-25 11:28:52 PST
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
Comment 6 Build Bot 2013-02-25 11:31:30 PST
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
Comment 7 Noam Rosenthal 2013-02-26 00:54:18 PST
Created attachment 190230 [details]
Patch
Comment 8 EFL EWS Bot 2013-02-26 01:11:28 PST
Comment on attachment 190230 [details]
Patch

Attachment 190230 [details] did not pass efl-ews (efl):
Output: http://webkit-commit-queue.appspot.com/results/16759777
Comment 9 Noam Rosenthal 2013-02-26 01:14:02 PST
Created attachment 190236 [details]
Patch
Comment 10 Noam Rosenthal 2013-02-26 02:08:27 PST
Created attachment 190244 [details]
Patch
Comment 11 Build Bot 2013-02-26 03:23:49 PST
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
Comment 12 Build Bot 2013-02-26 04:05:18 PST
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
Comment 13 Noam Rosenthal 2013-02-26 04:57:35 PST
Created attachment 190262 [details]
Patch
Comment 14 Kenneth Rohde Christiansen 2013-02-26 05:29:32 PST
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?
Comment 15 Noam Rosenthal 2013-02-26 05:32:50 PST
(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.
Comment 16 Kenneth Rohde Christiansen 2013-02-26 07:01:55 PST
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 && :-)
Comment 17 Noam Rosenthal 2013-02-26 08:02:54 PST
Created attachment 190285 [details]
Patch
Comment 18 Allan Sandfeld Jensen 2013-02-26 11:32:05 PST
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?
Comment 19 Noam Rosenthal 2013-02-26 12:51:04 PST
Created attachment 190343 [details]
Patch
Comment 20 WebKit Review Bot 2013-02-26 15:43:23 PST
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
Comment 21 Noam Rosenthal 2013-02-26 23:41:21 PST
Created attachment 190453 [details]
Patch
Comment 22 WebKit Review Bot 2013-02-27 01:41:12 PST
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
Comment 23 Noam Rosenthal 2013-02-27 01:45:41 PST
Created attachment 190467 [details]
Patch
Comment 24 Allan Sandfeld Jensen 2013-02-27 08:05:48 PST
Comment on attachment 190467 [details]
Patch

r=me
Comment 25 WebKit Review Bot 2013-02-27 08:18:12 PST
Comment on attachment 190467 [details]
Patch

Clearing flags on attachment: 190467

Committed r144190: <http://trac.webkit.org/changeset/144190>
Comment 26 WebKit Review Bot 2013-02-27 08:18:20 PST
All reviewed patches have been landed.  Closing bug.