[Webkit-unassigned] [Bug 76336] [Texmap] TextureMapper creates two many big intermediate surfaces

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Jan 16 05:35:05 PST 2012


https://bugs.webkit.org/show_bug.cgi?id=76336





--- Comment #7 from Jocelyn Turcotte <jocelyn.turcotte at nokia.com>  2012-01-16 05:35:05 PST ---
(From update of attachment 122566)
View in context: https://bugs.webkit.org/attachment.cgi?id=122566&action=review

> Source/WebCore/platform/graphics/texmap/TextureMapper.cpp:34
> +        // If the surface has only one reference, we can safely reuse it.

Please add something like "... only one reference (the reference of m_texturePool), we can...".

> Source/WebCore/platform/graphics/texmap/TextureMapperNode.cpp:285
> +    TransformationMatrix transform =
> +            TransformationMatrix(options.transform)
> +            .multiply(m_transform.combined())
> +            .translate(options.offset.width(), options.offset.height());

Since we now compute the transforms just before painting, it feels wasted to do:
- Compute the transforms for the whole graphic layer tree before we start painting.
- During painting, possibly reverse a whole part of the transformed branch in case we are rendering to an intermediate surface.

So it might be simpler and more efficient to integrate computeTransformsRecursive into paintSelf and decide if we combine with the parent transform or not. I'm not sure how much work that would be so this could be done in another patch.

> Source/WebCore/platform/graphics/texmap/TextureMapperNode.cpp:422
> +        replicaOptions.transform
> +                  .multiply(m_state.replicaLayer->m_transform.combined())
> +                  .multiply(m_transform.combined().inverse());

If I understand well I think that "replicaOptions.transform = TransformationMatrix(m_state.replicaLayer->m_transform.combined()).multiply(m_transform.combined().inverse())" would be clearer and would avoid an extra TransformationMatrix::multiply call.

> Source/WebCore/platform/graphics/texmap/TextureMapperNode.cpp:453
> +    paintOptions.transform = m_transform.combinedForChildren().inverse();

Humm, this is stretching my brain a bit, but I think that this should be m_transform.combined().inverse(), else this would revert the perspective transform in combinedForChildren which needs to act on the Z component, and rendering on the intermediate surface will flatten the Z.

> Source/WebCore/platform/graphics/texmap/TextureMapperNode.cpp:464
> +    TransformationMatrix targetTransform = TransformationMatrix().multiply(options.transform).multiply(m_transform.combined()).translate(options.offset.width(), options.offset.height());
> +    options.textureMapper->drawTexture(*surface.get(), surfaceRect, targetTransform, options.opacity, maskTexture.get());

Same thing here, TransformationMatrix().multiply(options.transform) can be simplified to TransformationMatrix(options.transform).

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.



More information about the webkit-unassigned mailing list