978980 - change CreatePathBuilder so it no longer requires a drawTarget object

Status: RESOLVED WONTFIX

(Core :: Graphics, defect)

Comment 1

[Rik Cabanier]

Attachment: Move createPathBuilder logic to static function + made exisisting methods call the static functions

Comment 2

[Rik Cabanier]

Comment on attachment 8385106 [details] [diff] [review]
Move createPathBuilder logic to static function + made exisisting methods call the static functions

Does this look reasonable? Should I add a platform function that creates a pathbuilder for the current platform?

Comment 3

[Rik Cabanier]

Try run: https://tbpl.mozilla.org/?tree=Try&rev=92e8dc02e801

Comment 4

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8385106 [details] [diff] [review]
Move createPathBuilder logic to static function + made exisisting methods call the static functions

Review of attachment 8385106 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me. Bas are you ok with this as well?

Comment 5

[Bas Schouten (:bas.schouten)]

Comment on attachment 8385106 [details] [diff] [review]
Move createPathBuilder logic to static function + made exisisting methods call the static functions

Review of attachment 8385106 [details] [diff] [review]:
-----------------------------------------------------------------

I feel this all gets really fiddly, we also have to figure out when we need to convert and when we actually end up with the 'expected' type of DrawTarget. And it prevents us from possibly in the future having DT-specific PathBuilders, which may be desirable at some point, I suggest one of two solutions:

1) Require a context for path creation.
2) Put a storage class for PathData inside Canvas that can stream into a DT-specific PathBuilder

Comment 6

[Rik Cabanier]

(In reply to Bas Schouten (:bas.schouten) from comment #5)
> Comment on attachment 8385106 [details] [diff] [review]
> Move createPathBuilder logic to static function + made exisisting methods
> call the static functions
> 
> Review of attachment 8385106 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I feel this all gets really fiddly, we also have to figure out when we need
> to convert and when we actually end up with the 'expected' type of
> DrawTarget. And it prevents us from possibly in the future having
> DT-specific PathBuilders, which may be desirable at some point,

Aren't pathbuilders already DT specific?

> I suggest
> one of two solutions:
> 
> 1) Require a context for path creation.

Do you mean a canvas context? 
The path object is already exposed in WebKit and Chrome with constructors.
Also, a path object can be moved between canvas objects with different backends and (in the future) SVG.

> 2) Put a storage class for PathData inside Canvas that can stream into a
> DT-specific PathBuilder

That was done in the path class itself. 
See this patch https://bugzilla.mozilla.org/page.cgi?id=splinter.html&bug=830734&attachment=8383387 :
          if (gfxpath->GetBackendType() == mTarget->GetType()) {
	    return gfxpath;
	  }
	 
	  RefPtr<gfx::PathBuilder> builder = mTarget->CreatePathBuilder(gfxpath->GetFillRule());
	  
	  gfxpath->StreamToSink(builder);
	  return builder->Finish();