WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
222758
WebKit must treat 'webgl' and 'webgl2' as distinct context types
https://bugs.webkit.org/show_bug.cgi?id=222758
Summary
WebKit must treat 'webgl' and 'webgl2' as distinct context types
Kenneth Russell
Reported
2021-03-04 12:22:35 PST
The wording isn't completely obvious in the specification:
https://html.spec.whatwg.org/multipage/canvas.html#dom-canvas-getcontext
but the 'webgl' and 'webgl2' context types must be treated as distinct from each other. If 'webgl' has been fetched for a canvas, then attempting to fetch 'webgl2' must return null. If 'webgl2' has been fetched for a canvas, then attempting to fetch 'webgl' must return null. This is the behavior in Firefox and Chrome. This Emscripten bug covers it:
https://github.com/emscripten-core/emscripten/pull/13497
Will add a WebGL conformance test for this at the same time as fixing it in WebKit.
Attachments
Patch
(10.07 KB, patch)
2021-05-04 17:54 PDT
,
Kenneth Russell
no flags
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Kenneth Russell
Comment 1
2021-03-10 13:48:02 PST
Haven't started this yet - unassigning until I can, in case someone else can pick it up. Should be a small fix, but important.
Radar WebKit Bug Importer
Comment 2
2021-03-11 12:23:13 PST
<
rdar://problem/75327781
>
Kenneth Russell
Comment 3
2021-05-04 17:54:09 PDT
Created
attachment 427717
[details]
Patch
EWS
Comment 4
2021-05-04 18:23:24 PDT
Committed
r276999
(
237319@main
): <
https://commits.webkit.org/237319@main
> All reviewed patches have been landed. Closing bug and clearing flags on
attachment 427717
[details]
.
Darin Adler
Comment 5
2021-05-04 19:57:00 PDT
Comment on
attachment 427717
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=427717&action=review
> Source/WebCore/html/HTMLCanvasElement.cpp:266 > + if (version == WebGLVersion::WebGL1 && !m_context->isWebGL1()) > + return Optional<RenderingContext> { WTF::nullopt }; > + if (version != WebGLVersion::WebGL1 && m_context->isWebGL1()) > + return Optional<RenderingContext> { WTF::nullopt };
I would suggest writing this instead: if ((version == WebGLVersion::WebGL1) != m_context->isWebGL1()) return Optional<RenderingContext> { };
> Source/WebCore/html/HTMLCanvasElement.cpp:498 > + if (type == WebGLVersion::WebGL1 && !m_context->isWebGL1()) > + return nullptr; > + > + if (type != WebGLVersion::WebGL1 && m_context->isWebGL1()) > + return nullptr;
Same here: if ((type == WebGLVersion::WebGL1) != m_context->isWebGL1()) return nullptr;
Kenneth Russell
Comment 6
2021-05-10 14:35:17 PDT
(In reply to Darin Adler from
comment #5
)
> Comment on
attachment 427717
[details]
> Patch > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=427717&action=review
> > > Source/WebCore/html/HTMLCanvasElement.cpp:266 > > + if (version == WebGLVersion::WebGL1 && !m_context->isWebGL1()) > > + return Optional<RenderingContext> { WTF::nullopt }; > > + if (version != WebGLVersion::WebGL1 && m_context->isWebGL1()) > > + return Optional<RenderingContext> { WTF::nullopt }; > > I would suggest writing this instead: > > if ((version == WebGLVersion::WebGL1) != m_context->isWebGL1()) > return Optional<RenderingContext> { }; > > > Source/WebCore/html/HTMLCanvasElement.cpp:498 > > + if (type == WebGLVersion::WebGL1 && !m_context->isWebGL1()) > > + return nullptr; > > + > > + if (type != WebGLVersion::WebGL1 && m_context->isWebGL1()) > > + return nullptr; > > Same here: > > if ((type == WebGLVersion::WebGL1) != m_context->isWebGL1()) > return nullptr;
Thanks for the suggestions. I considered these as well as using the xor operator but thought the code would be more readable as written. Happy to update it if you feel strongly.
Darin Adler
Comment 7
2021-05-10 14:52:50 PDT
(In reply to Kenneth Russell from
comment #6
)
> Thanks for the suggestions. I considered these as well as using the xor > operator but thought the code would be more readable as written. Happy to > update it if you feel strongly.
I think using xor is super-unreadable, oblique tricky-programmer Boolean logic. But using != seems *more* direct to me than the multiple if statements and likely *more* readable. Could use names if you feel that makes a big difference: bool versionIsWebGL1 = version == WebGLVersion::WebGL1; if (versionIsWebGL1 != m_context->isWebGL1()) return Optional<RenderingContext> { }; Or the boolean name could be requestingWebGL1. The point of the code, is "if we have the wrong type", which is best coded as "type != the needed type".
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug