816176 - Move WebGLShaderPrecisionFormat to a separate header

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

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

This one should be easier than other "Move WebGLFoo into separate files" bugs, because this class is particularly small.

Currently it lives in WebGLContext.h. Move it to a new header, WebGLShaderPrecisionFormat.h, which will have to include WebGLObjectModel.h.

If you're bored, look at other "Move WebGLFoo into separate files" bugs --- I can file more as existing ones get closed --- but I do appreciate work on all the boring little things ;-)

Comment 1

[Saurabh Anand [:sawrubh]]

Attachment: Patch v1

Comment 2

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

Comment on attachment 687366 [details] [diff] [review]
Patch v1

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

Thanks!

For a bigger challenge, do you feel like doing WebGLShader (Bug 816181) next?

Comment 3

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

I get this compile error:

/hack/mozilla-inbound/content/canvas/src/WebGLContextGL.cpp: In member function ‘JS::Value mozilla::WebGLContext::GetParameter(JSContext*, WebGLenum, mozilla::ErrorResult&)’:
/hack/mozilla-inbound/content/canvas/src/WebGLContextGL.cpp:1888:39: error: ‘MINVALUE_GL_MAX_RENDERBUFFER_SIZE’ was not declared in this scope

Comment 4

[Saurabh Anand [:sawrubh]]

It was because the WebGLContext.h is under constant flux with the recent "Move/Split Foo into Bar" bugs. Hence when I submitted this patch for review, it was building fine with the code I had (and I had most probably pulled the latest changes and tested with that) however I think some other patch must have landed in the meanwhile which has touched the same files. In fact when I pulled again and then tried to apply this patch, one HUNK failed to apply.

Possible solutions :
1) I repost the un-bitrotted patch and then we land it *quickly* before anyone else touches it again.
2) We can either have a single person working on these or maybe make people work sequentially on these type of bugs.

Thanks.

Comment 5

[Saurabh Anand [:sawrubh]]

Attachment: Patch v2

This is the un-bitrotted patch.

Note : I am setting the r+ myself because there was is no change from Patch v1 and Patch v1 had bjacob's r+.

Comment 6

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/253009438c5b

Comment 7

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

Thanks for rebasing & landing. Such bitrotting happens frequently, no need to serialize work. Next time I guess I'll just rebase and land.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/253009438c5b

Comment 9

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

Don't obsolete the patch that I reviewed. Otherwise it looks like you reviewed your own patch :)