[Webkit-unassigned] [Bug 63290] Stack overflow with enormous SVG filter

[bugzilla-daemon at webkit.org]

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





--- Comment #22 from Dean Jackson <dino at apple.com>  2014-09-25 18:39:10 PST ---
(From update of attachment 238630)
View in context: https://bugs.webkit.org/attachment.cgi?id=238630&action=review

>>> Source/WebCore/platform/graphics/filters/FilterEffect.cpp:113
>>> +        depth = std::max(depth, in->depthOfEffectInputs());
>> 
>> Just a little misunderstanding I suppose. Not the depth matters, but the amount of effects matters. So just sum up all effects.
> 
> I am not sure I understand your point here.  Summing up all the effects will get us the total number of effects in the filter tree.  But we already have a cut off condition on the total number of effects in the filter map to be < 200.  Why do we need to limit the number of the used effects by the filter to be < 100?  I thought we care about the depth because it represents how much complex the filter composition will be.  But the total number of effects used by the filter does not reveal this information.

What Dirk is saying is that only active effects will be added to the map. So, we could have less than 200 children in the <filter> but only 80 of them are connected into the chain/graph. Now, when we come to actually process the overall effect, each one of those in the map will contribute to the result, so it is the total number here that counts rather than the maximum depth.

For example, I could have 150 separate feColorMatrix elements, all manipulating the SourceGraphic, and then combined with feMerge. The maximum depth is 2, but we've got more than 100 effects.


<filter>
  <feColorMatrix in="SourceGraphic" out="output1" type="matrix" values="0.001 0 0 0 0 0 0.001 0 0 0 0 0 0.001 0 0 0 0 0 0.001 0"/>
  <feColorMatrix in="SourceGraphic" out="output2" type="matrix" values="0.001 0 0 0 0 0 0.001 0 0 0 0 0 0.001 0 0 0 0 0 0.001 0"/>
  <feColorMatrix in="SourceGraphic" out="output3" type="matrix" values="0.001 0 0 0 0 0 0.001 0 0 0 0 0 0.001 0 0 0 0 0 0.001 0"/>
  <feColorMatrix in="SourceGraphic" out="output4" type="matrix" values="0.001 0 0 0 0 0 0.001 0 0 0 0 0 0.001 0 0 0 0 0 0.001 0"/>
...
  <feMerge>
    <feMergeNode in="out1"/>
    <feMergeNode in="out2"/>
    <feMergeNode in="out3"/>
    <feMergeNode in="out4"/>
    ...
  </feMerge>
</filter>

>>> Source/WebCore/rendering/svg/RenderSVGResourceFilter.h:90
>>> +    const unsigned s_maxCountOfInputEffects = 200; // maximum number of input effects regardless whether they are connected to a filter's lastEffect or not
>> 
>> Please use sentences as comments with periods. Move the comment a line up before the constant. Also, can't it be static constant? You already use s_.
> 
> 

I think this, and s_maxDepthOfInputEffects, should just be file static in the .cpp. No need to put them on the class.

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