Hoist paint-server transform adaption out of preparePaintServer

Hoist paint-server transform adaption out of preparePaintServer

The additional transform computed by the helper function
RenderSVGResourceContainer::computeResourceSpaceTransform by each
paint-server (that is CTM-dependent), only depends on the input to
preparePaintServer, and not the paint-server itself.
Hoist the computation into SVGRenderSupport::updateGraphicsContext.
This in turn mean that the various preparePaintServer implementations
no longer use the passed RenderStyle (previously only used for checking
for vector-effect) and RenderSVGResourceModeFlags, so remove those.
computeResourceSpaceTransform w/ associated helpers are moved to
SVGRenderSupport.

BUG=420022
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=183746

[fs, 2014-10-14 13:10:36]

fs@opera.com changed reviewers:
+ fmalita@chromium.org, pdr@chromium.org, schenney@chromium.org

[fs, 2014-10-14 13:49:17]

https://codereview.chromium.org/652243002/diff/1/Source/core/rendering/svg/SV...
File Source/core/rendering/svg/SVGRenderSupport.cpp (right):

https://codereview.chromium.org/652243002/diff/1/Source/core/rendering/svg/SV...
Source/core/rendering/svg/SVGRenderSupport.cpp:398: static bool
shouldTransformOnTextPainting(RenderObject& renderer, AffineTransform&
resourceTransform)
Will remove these three functions in the next CL. Split it this way in order to
increase CL-parallelism. If https://codereview.chromium.org/654873002/ lands
smoothly I could merge the follow-up with this CL - I don't think the
combination increases the cognitive load that much. Let me know if you prefer
one way or the other.

[pdr., 2014-10-14 23:38:48]

LGTM

I'm a little sad that gradient/pattern transform updating is now spread across
two places. For example, we need to be careful when the gradient/pattern
transform is accessed that the appropriate transforms have been applied. That
said, I think this kind of disentangling is what's going to be needed to clean
up this squirrely code.

https://codereview.chromium.org/652243002/diff/1/Source/core/rendering/svg/SV...
File Source/core/rendering/svg/SVGRenderSupport.cpp (right):

https://codereview.chromium.org/652243002/diff/1/Source/core/rendering/svg/SV...
Source/core/rendering/svg/SVGRenderSupport.cpp:398: static bool
shouldTransformOnTextPainting(RenderObject& renderer, AffineTransform&
resourceTransform)
On 2014/10/14 at 13:49:17, fs wrote:
> Will remove these three functions in the next CL. Split it this way in order
to increase CL-parallelism. If https://codereview.chromium.org/654873002/ lands
smoothly I could merge the follow-up with this CL - I don't think the
combination increases the cognitive load that much. Let me know if you prefer
one way or the other.

SGTM.

Nit: The crazy comment below (... "Cmp. RenderObject.h's is*() methods.") can
probably just be removed.

[fs, 2014-10-15 10:52:20]

On 2014/10/14 23:38:48, pdr wrote:
> LGTM
> 
> I'm a little sad that gradient/pattern transform updating is now spread across
> two places. For example, we need to be careful when the gradient/pattern
> transform is accessed that the appropriate transforms have been applied. That
> said, I think this kind of disentangling is what's going to be needed to clean
> up this squirrely code.

Yes, it's a bit unfortunate, but it's technically no different from before.
Preferably these adjustment should be applied when updating GC state I think.
(This is after all just an ad-hoc/manual way to say "I want this paint-server to
use this CTM".)

>
https://codereview.chromium.org/652243002/diff/1/Source/core/rendering/svg/SV...
> File Source/core/rendering/svg/SVGRenderSupport.cpp (right):
> 
>
https://codereview.chromium.org/652243002/diff/1/Source/core/rendering/svg/SV...
> Source/core/rendering/svg/SVGRenderSupport.cpp:398: static bool
> shouldTransformOnTextPainting(RenderObject& renderer, AffineTransform&
> resourceTransform)
> On 2014/10/14 at 13:49:17, fs wrote:
> > Will remove these three functions in the next CL. Split it this way in order
> to increase CL-parallelism. If https://codereview.chromium.org/654873002/
lands
> smoothly I could merge the follow-up with this CL - I don't think the
> combination increases the cognitive load that much. Let me know if you prefer
> one way or the other.
> 
> SGTM.

Not sure what sounded good to you, but I'll land this and upload the second part
separately =)
 
> Nit: The crazy comment below (... "Cmp. RenderObject.h's is*() methods.") can
> probably just be removed.

...and the second part will remove this comment.

[fs, 2014-10-15 10:52:35]

The CQ bit was checked by fs@opera.com

[commit-bot: I haz the power, 2014-10-15 10:53:03]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/652243002/1

[commit-bot: I haz the power, 2014-10-15 13:03:51]

Committed patchset #1 (id:1) as 183746