1188540 - Forward texture-related calls to WebGLTexture

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

[Kelsey Gilbert [:jgilbert]]

Instead of polluting our massive WebGLContextGL.cpp file, we can forward these to a sensible place, much like the forwarding we have for shaders, programs, and other objects.

Comment 1

[Kelsey Gilbert [:jgilbert]]

Attachment: 0001-Move-member-decls-to-top-of-WebGLTexture.patch

Comment 2

[Kelsey Gilbert [:jgilbert]]

Attachment: 0002-Forward-and-translate.patch

This is mostly moves. Only code changes are necessary translations.

Comment 3

[Kelsey Gilbert [:jgilbert]]

Attachment: 0003-Forward-webgl2-texture-uploads.patch

Same, but for WebGL2 funcs.

Comment 4

[Kelsey Gilbert [:jgilbert]]

Oh, I forgot I also normalized a bunch of variable names.

Comment 5

[Kelsey Gilbert [:jgilbert]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d5d766860efd

Comment 6

[u480271]

Comment on attachment 8640017 [details] [diff] [review]
0002-Forward-and-translate.patch

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

::: dom/canvas/WebGL2ContextTextures.cpp
@@ +396,4 @@
>          return ErrorInvalidOperation("texSubImage2D: not enough data for operation (need %d, have %d)", bytesNeeded, dataLength);
>  
>      if (imageInfo.HasUninitializedImageData()) {
> +        bool coversWholeImage = xOffset == 0 &&

Wrap with (). (Helps emacs auto-format)

::: dom/canvas/WebGLTexture.h
@@ +86,4 @@
>      ~WebGLTexture() {
>          DeleteOnce();
>      }
> +public:

blank line above

Comment 7

[Kelsey Gilbert [:jgilbert]]

Attachment: 0004-Build-with-warn-as-error-on-Linux.patch

Comment 8

[Kelsey Gilbert [:jgilbert]]

Attachment: 0005-Build-on-linux-after-rebase.patch

Comment 9

[Kelsey Gilbert [:jgilbert]]

LGTM: https://treeherder.mozilla.org/#/jobs?repo=try&revision=4e1f9299a309

Comment 10

[u480271]

Comment on attachment 8641836 [details] [diff] [review]
0004-Build-with-warn-as-error-on-Linux.patch

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

Maybe we should take :smaug's advice and put the DOM facing WebGL stuff into the dom namespace at some point.  Would be happy to keep implementation details in webgl:: but anything that is called via .webidl bindings would be simpler in dom::. It would clean up dom/bindings/Bindings.conf too.

Comment 11

[Kelsey Gilbert [:jgilbert]]

(In reply to Dan Glastonbury :djg :kamidphish from comment #10)
> Comment on attachment 8641836 [details] [diff] [review]
> 0004-Build-with-warn-as-error-on-Linux.patch
> 
> Review of attachment 8641836 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Maybe we should take :smaug's advice and put the DOM facing WebGL stuff into
> the dom namespace at some point.  Would be happy to keep implementation
> details in webgl:: but anything that is called via .webidl bindings would be
> simpler in dom::. It would clean up dom/bindings/Bindings.conf too.

Something to consider, though much of WebGL would end up as dom:: then.

Comment 12

[Pulsebot]

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

Comment 13

[Carsten Book [:Tomcat]]

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