Gallery: ImageProcessor.prototype.draw should not call bufferData everytime

RESOLVED FIXED

Status

Firefox OS
Gaia::Gallery
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: bjacob, Assigned: bjacob)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

This is quite similar to bug 934790. Less critical, because a small WebGL buffer is cheaper to allocate than a larger WebGL texture, but still. There is no reason to reallocate the WebGL buffer storing the drawing geometry everytime. It also shouldn't depend on the parameters passed to draw(). Instead, just create once and for all a dummy buffer with the geometry of a dummy rectangle, and use uniforms to control its actual position and dimensions.
(Assignee)

Comment 1

4 years ago
Also, only one buffer is needed instead of two, and it only needs 4 vertices with a TRIANGLE_STRIP, not 6.
(Assignee)

Comment 2

4 years ago
Created attachment 827786 [details] [diff] [review]
Don't upload buffers on every draw

Jeff: please review from WebGL correctness perspective. This patch should not make any rendering difference; if it somehow can, that's a bug.

David: please review from Gallery perspective: this patch should not change anything in Gallery app besides how it uses WebGL.
Attachment #827786 - Flags: review?(jgilbert)
Attachment #827786 - Flags: review?(dflanagan)
Comment on attachment 827786 [details] [diff] [review]
Don't upload buffers on every draw

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

::: apps/gallery/js/ImageEditor.js
@@ +1347,5 @@
>  
>    // Create buffers to hold the input and output rectangles
> +  this.rectangleBuffer = gl.createBuffer();
> +  gl.bindBuffer(gl.ARRAY_BUFFER, this.rectangleBuffer);
> +  gl.disable(gl.CULL_FACE);

Why do you disable CULL_FACE here in the middle of buffer stuff?
Attachment #827786 - Flags: review?(jgilbert) → review+
(Assignee)

Comment 4

4 years ago
Oops, that was of course a remnant from debugging, sorry.
(Assignee)

Comment 5

4 years ago
Created attachment 8340644 [details] [diff] [review]
Don't upload buffers on every draw v2 (ready for landing)

Removed the unwanted gl.disable(gl.CULL_FACE);
Attachment #8340644 - Flags: review+
(Assignee)

Updated

4 years ago
Attachment #827786 - Flags: review?(dflanagan)
(Assignee)

Updated

4 years ago
Attachment #8340644 - Flags: review?(dflanagan)
Rolled this patch up with two others and resolved it in the parent bug.
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED

Updated

4 years ago
Attachment #8340644 - Flags: review?(dflanagan)

Updated

4 years ago
Assignee: nobody → bjacob
You need to log in before you can comment on or make changes to this bug.