Closed
Bug 1080137
Opened 10 years ago
Closed 10 years ago
WebGL2: misc fixes to make new tex formats and sized internalformats actually work
Categories
(Core :: Graphics: CanvasWebGL, defect)
Tracking
()
RESOLVED
FIXED
mozilla35
People
(Reporter: bjacob, Assigned: bjacob)
References
Details
Attachments
(2 files)
31.83 KB,
patch
|
jgilbert
:
review+
|
Details | Diff | Splinter Review |
25.51 KB,
patch
|
jgilbert
:
review+
|
Details | Diff | Splinter Review |
This is the collection of all the misc stuff that I had to fix to make along with making these new conformance tests, https://github.com/KhronosGroup/WebGL/pull/736 https://github.com/KhronosGroup/WebGL/pull/735 What we have here ranges from the expected (adjusting validation logic to accept things that are legal in WebGL2) to the silly (typos in the big formats table). There is some code removal: BasTexFormat goes away, it had become redundant, and had only one caller left, and that caller was not useful. The only nontrivial part here was how to deal with new formats without having to write all the code to support texture format conversions for them, or alpha premultiplication, before we even know what the WG will settle on on this topic. This is resolved by introducing a dummy WebGLTexelFormat (which is an enum used solely around tex conversions, so maybe it should be renamed to reflect that) saying "I'm a format for which no conversion is supported" and generate INVALID_OPERATION when trying to actually convert those.
Attachment #8502030 -
Flags: review?(jgilbert)
Assignee | ||
Comment 1•10 years ago
|
||
Comment on attachment 8502030 [details] [diff] [review] support-new-formats Review of attachment 8502030 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/canvas/WebGLContextGL.cpp @@ +3729,5 @@ > > + WebGLTexelFormat dstFormat = GetWebGLTexelFormat(effectiveinternalformat); > + if (dstFormat == WebGLTexelFormat::FormatNotSupportingAnyConversion) { > + if (srcFormat != WebGLTexelFormat::FormatNotSupportingAnyConversion && > + srcFormat != WebGLTexelFormat::Auto) I'm being silly here. This if condition should be simply: if (srcFormat != WebGLTexelFormat::Auto)
Comment 2•10 years ago
|
||
Comment on attachment 8502030 [details] [diff] [review] support-new-formats Review of attachment 8502030 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/canvas/WebGLContextGL.cpp @@ +3689,5 @@ > + // EffectiveInternalFormatFromInternalFormatAndType for that purpose. Really, an unsized source format > + // is the same thing as an unsized internalformat. > + TexInternalFormat effectivesourceformat = > + EffectiveInternalFormatFromInternalFormatAndType(format, type); > + MOZ_ASSERT(effectivesourceformat != LOCAL_GL_NONE); // should have validated format/type combo earlier Camel-case, please. @@ +3896,5 @@ > } > > + const size_t bitspertexel = GetBitsPerTexel(existingEffectiveInternalFormat); > + MOZ_ASSERT((bitspertexel % 8) == 0); // should not have compressed formats here. > + const size_t srcTexelSize = bitspertexel / 8; I like this new stuff.
Attachment #8502030 -
Flags: review?(jgilbert) → review+
Assignee | ||
Comment 3•10 years ago
|
||
Attachment #8502195 -
Flags: review?(jgilbert)
Comment 4•10 years ago
|
||
Comment on attachment 8502195 [details] [diff] [review] address review comment, make fixes needed to still pass WebGL1 conformance tests, remove depth formats from WebGLTexelFormat as they don't support conversions Review of attachment 8502195 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/canvas/WebGLContextGL.cpp @@ +3736,4 @@ > WebGLTexelFormat actualSrcFormat = srcFormat == WebGLTexelFormat::Auto ? dstFormat : srcFormat; > > if (byteLength) { > + size_t bitspertexel = GetBitsPerTexel(effectiveInternalFormat); bitsPerTexel
Attachment #8502195 -
Flags: review?(jgilbert) → review+
Assignee | ||
Comment 5•10 years ago
|
||
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=f54a890d208f
Assignee | ||
Comment 6•10 years ago
|
||
Surprisingly tricky to land: this patch was trying to always pass sized internalformats to the driver, but that turned out to make our test slaves unhappy with old 16bit formats. Now passing sized internalformats only for the new formats introduced in WebGL2 for which unsized internalformat is illegal. https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=9167008f11d7
Assignee | ||
Comment 7•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/367b155c5b5e
Assignee: nobody → bjacob
https://hg.mozilla.org/mozilla-central/rev/367b155c5b5e
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
You need to log in
before you can comment on or make changes to this bug.
Description
•