Review commit: https://reviewboard.mozilla.org/r/62208/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/62208/

Review commit: https://reviewboard.mozilla.org/r/62210/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/62210/

Review commit: https://reviewboard.mozilla.org/r/62212/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/62212/

This one has issues still: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9957c03a5e77&selectedJob=23334278

Comment on attachment 8767793 [details] Bug 1281098 - Fix UNPACK_ handling. - Review request updated; see interdiff: https://reviewboard.mozilla.org/r/62208/diff/1-2/

Comment on attachment 8767794 [details] Bug 1281098 - Disallow querying texture swizzle. - Review request updated; see interdiff: https://reviewboard.mozilla.org/r/62210/diff/1-2/

Comment on attachment 8767795 [details] Bug 1281098 - Cache LOCAL_GL_TEXTURE_COMPARE_MODE since we need it for validation. - Review request updated; see interdiff: https://reviewboard.mozilla.org/r/62212/diff/1-2/

Review commit: https://reviewboard.mozilla.org/r/62512/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/62512/

Comment on attachment 8767793 [details] Bug 1281098 - Fix UNPACK_ handling. - https://reviewboard.mozilla.org/r/62208/#review59436 ::: dom/canvas/WebGLTextureUpload.cpp:61 (Diff revision 2) > */ > > +static bool > +ValidateExtents(WebGLContext* webgl, const char* funcName, GLsizei width, GLsizei height, > + GLsizei depth, GLint border, uint32_t* const out_width, > + uint32_t* const out_height, uint32_t* const out_depth) As a general comment a lot of the function in this file have a lot of parameters. It would be better if these sets of parameters were packaged up into appropriate aggregates. This would make the method structures cleaner and would make returning Maybe<>s a better option than bags of out params. This makes it clearer which out params are set when. ::: dom/canvas/WebGLTextureUpload.cpp:267 (Diff revision 2) > + &width, &height, &depth)) > + { > + return; > + } > + > + webgl::PackingInfo pi; It'd be nice if we used a longer variable name than pi. Especially because when people see pi they usually think of 3.1415. ::: dom/canvas/WebGLTextureUpload.cpp:321 (Diff revision 2) > +WebGLContext::GetUnpackValuesForImage(const char* funcName, uint32_t srcImageWidth, > + uint32_t srcImageHeight, > + uint32_t* const out_rowLength, > + uint32_t* const out_imageHeight) > +{ > + uint32_t rowLength = mPixelStore_UnpackRowLength; How about the following instead: if (mPixelStore_UnpackRowLength && mPixelStore_UnpackRowLength != srcImageWidth) { ErrorInvalidOperation("..."); return false; } if (mPixelStore_UnpackImageHeight && mPixelStore_UnpackImageHeight != srcImageHeight) { ErrorInvalidOperation("..."); return false; } *out_rowLength = srcImageWidth; *out_imageHieght = srcImageHeight; I think that makes it more clear what's going on.

Comment on attachment 8767794 [details] Bug 1281098 - Disallow querying texture swizzle. - https://reviewboard.mozilla.org/r/62210/#review59594

Comment on attachment 8768210 [details] Bug 1281098 - Fix failure case. - https://reviewboard.mozilla.org/r/62512/#review59604

Comment on attachment 8767795 [details] Bug 1281098 - Cache LOCAL_GL_TEXTURE_COMPARE_MODE since we need it for validation. - https://reviewboard.mozilla.org/r/62212/#review59614

Pushed by jgilbert@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/a8bdde3cb347 Fix UNPACK_ handling. - r=jrmuizel https://hg.mozilla.org/integration/mozilla-inbound/rev/67394a97b3cf Disallow querying texture swizzle. - r=ethlin https://hg.mozilla.org/integration/mozilla-inbound/rev/27174fcdff4c Cache LOCAL_GL_TEXTURE_COMPARE_MODE since we need it for validation. - r=ethlin https://hg.mozilla.org/integration/mozilla-inbound/rev/57118eba003d Fix failure case.

https://hg.mozilla.org/mozilla-central/rev/a8bdde3cb347 https://hg.mozilla.org/mozilla-central/rev/67394a97b3cf https://hg.mozilla.org/mozilla-central/rev/27174fcdff4c https://hg.mozilla.org/mozilla-central/rev/57118eba003d