Closed
Bug 1075305
Opened 10 years ago
Closed 10 years ago
WebGL2 - Track GL internalformat in WebGLTexture::ImageInfo
Categories
(Core :: Graphics: CanvasWebGL, defect)
Core
Graphics: CanvasWebGL
Tracking
()
RESOLVED
FIXED
mozilla35
People
(Reporter: u480271, Assigned: u480271)
References
Details
Attachments
(4 files, 3 obsolete files)
4.92 KB,
patch
|
bjacob
:
review+
|
Details | Diff | Splinter Review |
21.26 KB,
patch
|
bjacob
:
review+
|
Details | Diff | Splinter Review |
19.12 KB,
patch
|
u480271
:
review+
|
Details | Diff | Splinter Review |
2.42 KB,
patch
|
bjacob
:
review+
|
Details | Diff | Splinter Review |
Track GL internalformat in WebGLTexture::ImageInfo
Attachment #8497939 -
Attachment is obsolete: true
Summary: WebGL2 - 3D Textures → WebGL2 - Track GL internalformat in WebGLTexture::ImageInfo
Add internal format tracking WebGLTexture::ImageInfo. This is a step on the path to sorting out the confusion. Once we have "effective internal format" tracking, format and type can be removed.
Attachment #8497945 -
Flags: review?(bjacob)
remove WebGL prefix from internalformat, format and type. It doesn't add anything but verbosity.
Attachment #8497947 -
Flags: review?(bjacob)
Attachment #8497949 -
Flags: review?(bjacob)
Comment 5•10 years ago
|
||
Comment on attachment 8497945 [details] [diff] [review] WebGL2 - Add internal format state to WebGLTexture::ImageInfo, in preparation for WebGL2 texture functions. Review of attachment 8497945 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/canvas/WebGLTexture.h @@ +75,5 @@ > > ImageInfo(GLsizei width, > GLsizei height, > + TexInternalFormat webGLInternalFormat, > + TexFormat webGLFormat, There's a contradiction between the title of this patch, and what it's doing here. As we can see here, we already have TexInternalFormat webGLFormat, indeed webGLFormat was converted to type TexInternalFormat in bug 1071340, a week ago, which you reviewed! So WebGLTexture::ImageInfo is _already_ storing a TexInternalFormat. There were a few instances left where we still confused that 'TexInternalFormat webGLFormat' for a non-internal format, and they are taken care of in bug 1072680. Why would you want to add a TexFormat field there? Non-internal texture formats are only used to describe incoming texture data in entry points such as texImage2D. Here, we are describing the actual format of the already-unpacked texture.
Attachment #8497945 -
Flags: review?(bjacob) → review-
Comment 6•10 years ago
|
||
As noted in bug 1072680 comment 8, it is unfortunate that the TexInternalFormat field is currently named "webGLFormat" suggesting that it's not an internalformat, but it really is an internalformat. A patch to just rename it to webGLInternalFormat would be r+.
Updated•10 years ago
|
Attachment #8497947 -
Flags: review?(bjacob) → review+
Updated•10 years ago
|
Attachment #8497949 -
Flags: review?(bjacob) → review+
(In reply to Benoit Jacob [:bjacob] from comment #6) > As noted in bug 1072680 comment 8, it is unfortunate that the > TexInternalFormat field is currently named "webGLFormat" suggesting that > it's not an internalformat, but it really is an internalformat. A patch to > just rename it to webGLInternalFormat would be r+. This is what I really want to do. I'll update the patch to rename format to internalformat.
Attachment #8498641 -
Flags: review?(bjacob)
Attachment #8497945 -
Attachment is obsolete: true
Attachment #8497947 -
Attachment is obsolete: true
Attachment #8498643 -
Flags: review?(bjacob) → review+
Assignee | ||
Comment 11•10 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=41662f6ff072
Assignee | ||
Comment 12•10 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=a5f47c3f198b
Assignee | ||
Comment 13•10 years ago
|
||
Attachment #8498713 -
Flags: review?(bjacob)
Comment 14•10 years ago
|
||
Comment on attachment 8498713 [details] [diff] [review] Sort strong GL type TexFormat. Review of attachment 8498713 [details] [diff] [review]: ----------------------------------------------------------------- Quite good idea, to put the numeric values in comments! I had been using this program to do the sorting: #include <iostream> #include <algorithm> #include <vector> #include "GLDefs.h" struct Val { int value; const char* name; Val(int v, const char* n) : value(v), name(n) {} bool operator<(const Val& other) const { return value < other.value; } bool operator==(const Val& other) const { return value == other.value; } }; int main() { std::vector<Val> values = { #define IGNORE // #define STRONG_GLENUM_VALUE(X) {LOCAL_GL_##X, #X} STRONG_GLENUM_VALUE(RGB), // pasted values to be sorted here #undef STRONG_GLENUM_VALUE }; std::sort(values.begin(), values.end()); for (auto i = values.begin(); i != values.end(); i++) { std::cout << " STRONG_GLENUM_VALUE(" << i->name << ")," << std::endl; } };
Attachment #8498713 -
Flags: review?(bjacob) → review+
Comment 15•10 years ago
|
||
Comment on attachment 8498641 [details] [diff] [review] Rename WebGLTexture::ImageInfo::WebGLFormat to WebGLTexture::ImageInfo::WebGLInternalFormat. Review of attachment 8498641 [details] [diff] [review]: ----------------------------------------------------------------- OK to land this, but this is going to be changed again with the effective-internalformats change.
Attachment #8498641 -
Flags: review?(bjacob) → review+
Assignee | ||
Comment 16•10 years ago
|
||
(In reply to Benoit Jacob [:bjacob] from comment #15) > Comment on attachment 8498641 [details] [diff] [review] > Rename WebGLTexture::ImageInfo::WebGLFormat to > WebGLTexture::ImageInfo::WebGLInternalFormat. > > Review of attachment 8498641 [details] [diff] [review]: > ----------------------------------------------------------------- > > OK to land this, but this is going to be changed again with the > effective-internalformats change. I understand and are in agreement with you.
Assignee | ||
Comment 17•10 years ago
|
||
(In reply to Dan Glastonbury :djg :kamidphish from comment #12) > https://tbpl.mozilla.org/?tree=Try&rev=a5f47c3f198b TexFormat strong type isn't sorted. Added patch.
Assignee | ||
Comment 18•10 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=dc805b06ca07
Comment 19•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/fdb83e28de39 https://hg.mozilla.org/mozilla-central/rev/08943f32886a https://hg.mozilla.org/mozilla-central/rev/138e0dba47e8
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Comment 20•10 years ago
|
||
And https://hg.mozilla.org/mozilla-central/rev/c65696202540 where you typoed the bug number
You need to log in
before you can comment on or make changes to this bug.
Description
•