Closed Bug 978980 Opened 6 years ago Closed 6 years ago

change CreatePathBuilder so it no longer requires a drawTarget object

Categories

(Core :: Graphics, defect)

defect
Not set

Tracking

()

RESOLVED WONTFIX

People

(Reporter: cabanier, Assigned: cabanier)

References

Details

Attachments

(1 file)

No description provided.
Blocks: 830734
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?
Attachment #8385106 - Flags: review?(jmuizelaar)
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?
Attachment #8385106 - Flags: superreview?(bas)
Attachment #8385106 - Flags: review?(jmuizelaar)
Attachment #8385106 - Flags: review+
Assignee: nobody → cabanier
Flags: needinfo?(bas)
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
Attachment #8385106 - Flags: superreview?(bas) → superreview-
(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();
Status: NEW → RESOLVED
Closed: 6 years ago
Flags: needinfo?(bas)
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.