Closed Bug 997014 Opened 6 years ago Closed 6 years ago

Remove nsICanvasRenderingContextInternal::Render

Categories

(Core :: Graphics, defect)

29 Branch
x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla31

People

(Reporter: mattwoodrow, Assigned: mattwoodrow)

References

Details

Attachments

(6 files)

This function heavily relies on Thebes, and we have GetSurfaceSnapshot as a Moz2D version already.
Assignee: nobody → matt.woodrow
This implements GetSurfaceSnapshot for WebGL and lets us indicate if the returned data is un-premultplied.
Attachment #8407312 - Flags: review?(roc)
The Moz2D UnpremultiplyDataSurface function here might conflict with your mutable data source surface changes.
Attachment #8407313 - Flags: review?(roc)
The comment in this file says it's meant to go away, so away it shall go.
Attachment #8407315 - Flags: review?(roc)
Apart from just converting to Moz2D, this also moves most of the complexity of dealing with unpremultiplied data out of SurfaceFromElement and into WebGL (the only place that uses it).
Attachment #8407316 - Flags: review?(roc)
Depends on: 996929
This code served a useful purpose when we had moz2d *and* thebes canvases, but now it's just a duplicate of SurfaceFromElement.
Attachment #8407375 - Flags: review?(roc)
Comment on attachment 8407312 [details] [diff] [review]
Part 1: Improve GetSurfaceSnapshot

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

::: content/canvas/public/nsICanvasRenderingContextInternal.h
@@ +93,5 @@
>    NS_IMETHOD GetThebesSurface(gfxASurface **surface) = 0;
>    
>    // This gets an Azure SourceSurface for the canvas, this will be a snapshot
>    // of the canvas at the time it was called.
> +  virtual mozilla::TemporaryRef<mozilla::gfx::SourceSurface> GetSurfaceSnapshot(uint32_t aFlags = 0, bool* aPremultAlpha = nullptr) = 0;

We don't need a separate flag, right? If aPremultAlpha is null, then we say the result surface must be premultiplied, otherwise we allow it to be premultiplied or not and set *aPremultAlpha to indicate which.

::: content/canvas/src/WebGLContext.cpp
@@ +1423,5 @@
> +    dt->DrawSurface(source,
> +                    Rect(0, 0, mWidth, mHeight),
> +                    Rect(0, 0, mWidth, mHeight),
> +                    DrawSurfaceOptions(),
> +                    DrawOptions(1.0f, CompositionOp::OP_SOURCE));

We don't have any code for in-place vertical flipping of a gfxImageSurface? Too bad.
Attachment #8407312 - Flags: review?(roc) → review+
sorry had to backout this changes for test failures like https://tbpl.mozilla.org/php/getParsedLog.php?id=37986531&tree=Mozilla-Inbound
You need to log in before you can comment on or make changes to this bug.