1038928 - WebGL element array cache: dont try to handle null out_upperBound, and add some test coverage for out_upperBound

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

[Benoit Jacob [:bjacob] (mostly away)]

Attachment: upper-bounds

out_upperBound is great, it's what allows us to use glDrawRangeElements to make webgl.drawElements faster. However there are two things that could be improved:

UpdateUpperBound checks whether out_upperBound is null all the time, even as in the WebGL implementation we always passs a non-null pointer. That seemed to be needed only to accomodate the unit test.

So instead, let's just assert that this pointer is non-null (saving some comparisons in hot code, should make us marginally slightly faster) and let's pass non-null pointers in the test, so that 1) it is closer to actual WebGL code, always a plus, and 2) we can make this test actually give some coverage to the out_upperBound values, checking that they always are <= maxValue in the success case, and always > maxValue in the error case.

Side changes:
 * some renaming of functions in the unit test, needed to make the name CheckValidate available
 * removed SetUpperBound, instead used UpdateUpperBound except for the first initialization where we just manually initialize.
 * In the WebGL impl, just initialize by 0 for the sake of not having uninitialized variables at all, dont bother initializing by UINT32_MAX, it's the job of the implementation and is now covered by tests, to properly set out_upperBound.

Comment 1

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8456450 [details] [diff] [review]
upper-bounds

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

::: content/canvas/src/WebGLContextDraw.cpp
@@ +303,5 @@
>  
>      if (!ValidateDrawModeEnum(mode, "drawElements: mode"))
>          return;
>  
> +    GLuint upperBound = 0;

Oops, good fix.

Comment 2

[Benoit Jacob [:bjacob] (mostly away)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/fad52f3c9132

Comment 3

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/fad52f3c9132

Comment 4

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/08c5f5e266c5

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

(In reply to Wes Kocher (:KWierso) from comment #4)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/08c5f5e266c5

This landed with an incorrect bug number. Should have been 1038929.

Comment 6

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/08c5f5e266c5

Comment 7

[Daniel Holbert [:dholbert]]

(comment 6 is also really for bug 1038929, per comment 5)