Closed
Bug 790712
Opened 12 years ago
Closed 12 years ago
Implement WEBGL_compressed_texture_atc
Categories
(Core :: Graphics: CanvasWebGL, defect)
Core
Graphics: CanvasWebGL
Tracking
()
RESOLVED
FIXED
mozilla18
People
(Reporter: bjacob, Assigned: bjacob)
References
(Blocks 1 open bug)
Details
(Whiteboard: webgl-extension webgl-next [games:p1] [LOE:S])
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
jgilbert
:
review+
|
Details | Diff | Splinter Review |
This patch hides this feature behind a preference, which is set on B2G only. So in principle, if needed, we can land it, as it's not actually enabled on any shipping product, but it's best if we can wait to first get the extension promoted to draft status. Working on it --- writing a conformance test. We'll see what to do by next week.
Assignee | ||
Updated•12 years ago
|
Severity: normal → critical
OS: Linux → All
Hardware: x86_64 → All
Comment 1•12 years ago
|
||
While this would be nice to land, it's not a v1 blocker. Re-nom if you disagree :)
blocking-basecamp: ? → -
Renomming -- It's essential for WebGL performance on the first target B2G devices (ATC is the only texture compression they will support)...
blocking-basecamp: - → ?
Comment 3•12 years ago
|
||
I spoke with Vlad about this and it's quite important, plus the work is almost ready to go (hence the LOE:S). We can always re-evaluate closer to the release on the off chance this didn't make it in by then.
Assignee: nobody → bjacob
blocking-basecamp: ? → +
Whiteboard: webgl-extension webgl-next [games:p1] → webgl-extension webgl-next [games:p1] [LOE:S]
Comment 4•12 years ago
|
||
Just to add a data point to this, the EA game developers on the WebGL thread are pretty specific that, with memory and performance limitations on mobile being what they are, compressed textures are essential to get anything but the most basic 2d or 3d game to work.
Assignee | ||
Comment 5•12 years ago
|
||
Updated: fixes an assert failure in DEBUG builds.
Attachment #660537 -
Attachment is obsolete: true
Just to be clear --- this is all in the realm of Khronos now, right? No more assistance needed from our friends?
Assignee | ||
Comment 7•12 years ago
|
||
Correct: the WebGL WG at Khronos will decide whether this can move to draft status, and we can help there by writing a nice conformance test, which I'm working on. No more help needed from any specific company. If we don't get this moved to draft status early enough, we will still have the option of landing this regardless, relying on this being preffed off on currently shipping products (Firefox), but that would be sub-optimal.
Assignee | ||
Comment 8•12 years ago
|
||
Promoted to draft status:
http://www.khronos.org/registry/webgl/extensions/WEBGL_compressed_texture_atc/
Can implement now.
Summary: Implement WEBGL_compressed_texture_atc (can't land until the spec proposal is promoted to draft) → Implement WEBGL_compressed_texture_atc
Assignee | ||
Updated•12 years ago
|
Attachment #661367 -
Flags: review?(jgilbert)
Assignee | ||
Comment 9•12 years ago
|
||
Attachment #661367 -
Attachment is obsolete: true
Attachment #661367 -
Flags: review?(jgilbert)
Attachment #663852 -
Flags: review?(jgilbert)
Comment 10•12 years ago
|
||
Comment on attachment 663852 [details] [diff] [review]
atc support
Review of attachment 663852 [details] [diff] [review]:
-----------------------------------------------------------------
A nit about vendor suffixes in our internal stuff. Non-compulsory, since we have tons of suffixed constants in there, but I'd like to move us away from that.
::: content/canvas/src/WebGLContextValidate.cpp
@@ +416,3 @@
> }
> + ErrorInvalidOperation("%s: level parameter does not match width and height", info);
> + return false;
Interesting use of switch and break for control flow. I like it.
::: gfx/gl/GLDefs.h
@@ +3093,5 @@
> #define LOCAL_WGL_CONTEXT_COMPATIBILITY_PROFILE_BIT_ARB 0x00000002
> #define LOCAL_WGL_CONTEXT_ROBUST_ACCESS_BIT_ARB 0x00000004
>
> +// AMD_compressed_ATC_texture
> +#define LOCAL_GL_ATC_RGB_AMD 0x8C92
I would prefer these without the vendor suffix. (Unless there is a conflict which would require using suffixes) It's fine to leave it like this, but these suffixes are really just unnecessary namespacing noise.
Attachment #663852 -
Flags: review?(jgilbert) → review+
Assignee | ||
Comment 11•12 years ago
|
||
Target Milestone: --- → mozilla18
Comment 12•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•