Bug 219256

Summary:

GraphicsContextGL should have robust multivalue setters

Product:

WebKit

Reporter:

Kimmo Kinnunen <kkinnunen>

Component:

WebGL

Assignee:

Kimmo Kinnunen <kkinnunen>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

cdumez, changseok, dino, esprehn+autocc, ews-watchlist, graouts, gyuyoung.kim, kondapallykalyan, pnormand, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Local Build

Hardware:

Unspecified

OS:

Unspecified

See Also:

https://bugs.webkit.org/show_bug.cgi?id=219340

Bug Depends on:

Bug Blocks:

217211, 219320

Attachments:

Description	Flags
Patch	none
Patch	ews-feeder: commit-queue-
Patch	none
Patch	ews-feeder: commit-queue-
Patch	ews-feeder: commit-queue-
Patch	none
Patch	none
Patch	ews-feeder: commit-queue-
Patch	ews-feeder: commit-queue-
Patch	ews-feeder: commit-queue-
Patch	none
Patch	none

Kimmo Kinnunen

Reported 2020-11-23 05:17:37 PST

GraphicsContextGL should have robust multivalue setters Instead of passing `size, ptr` we should pass `GCGLSpan(ptr, size)` in all functions taking buffer parameters.

Attachments
Patch (169.92 KB, patch) 2020-11-24 05:16 PST, Kimmo Kinnunen	no flags	Details Formatted Diff Diff
Patch (173.39 KB, patch) 2020-11-24 05:27 PST, Kimmo Kinnunen	ews-feeder: commit-queue-	Details Formatted Diff Diff
Patch (174.26 KB, patch) 2020-11-24 05:40 PST, Kimmo Kinnunen	no flags	Details Formatted Diff Diff
Patch (175.37 KB, patch) 2020-11-24 05:48 PST, Kimmo Kinnunen	ews-feeder: commit-queue-	Details Formatted Diff Diff
Patch (176.23 KB, patch) 2020-11-24 06:50 PST, Kimmo Kinnunen	ews-feeder: commit-queue-	Details Formatted Diff Diff
Patch (187.65 KB, patch) 2020-11-24 07:09 PST, Kimmo Kinnunen	no flags	Details Formatted Diff Diff
Patch (187.55 KB, patch) 2020-11-24 07:17 PST, Kimmo Kinnunen	no flags	Details Formatted Diff Diff
Patch (196.56 KB, patch) 2020-11-26 05:16 PST, Kimmo Kinnunen	ews-feeder: commit-queue-	Details Formatted Diff Diff
Patch (198.93 KB, patch) 2020-11-26 05:26 PST, Kimmo Kinnunen	ews-feeder: commit-queue-	Details Formatted Diff Diff
Patch (198.94 KB, patch) 2020-11-26 05:38 PST, Kimmo Kinnunen	ews-feeder: commit-queue-	Details Formatted Diff Diff
Patch (198.96 KB, patch) 2020-11-26 05:43 PST, Kimmo Kinnunen	no flags	Details Formatted Diff Diff
Patch (199.06 KB, patch) 2020-11-26 05:50 PST, Kimmo Kinnunen	no flags	Details Formatted Diff Diff
Show Obsolete (11) View All Add attachment proposed patch, testcase, etc.

Kimmo Kinnunen

Comment 1 2020-11-24 05:16:29 PST

Created attachment 414831 [details] Patch

Kimmo Kinnunen

Comment 2 2020-11-24 05:27:27 PST

Created attachment 414832 [details] Patch

Kimmo Kinnunen

Comment 3 2020-11-24 05:40:06 PST

Created attachment 414833 [details] Patch

Kimmo Kinnunen

Comment 4 2020-11-24 05:48:31 PST

Created attachment 414834 [details] Patch

Kimmo Kinnunen

Comment 5 2020-11-24 06:50:24 PST

Created attachment 414837 [details] Patch

Kimmo Kinnunen

Comment 6 2020-11-24 07:09:13 PST

Created attachment 414839 [details] Patch

Kimmo Kinnunen

Comment 7 2020-11-24 07:17:36 PST

Created attachment 414840 [details] Patch

Dean Jackson

Comment 8 2020-11-25 01:29:40 PST

Comment on attachment 414840 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=414840&action=review Very nice. > Source/WebCore/html/canvas/WebGL2RenderingContext.cpp:1132 > - m_context->getExtensions().texImage2DRobustANGLE(target, level, internalformat, width, height, border, format, type, 0, reinterpret_cast<void*>(offset)); > + m_context->texImage2D(target, level, internalformat, width, height, border, format, type, offset); Was this intentional? Does texImage2D call the robust ANGLE form itself?

Kimmo Kinnunen

Comment 9 2020-11-26 05:16:58 PST

Created attachment 414895 [details] Patch

Kimmo Kinnunen

Comment 10 2020-11-26 05:21:49 PST

(In reply to Dean Jackson from comment #8) > > Source/WebCore/html/canvas/WebGL2RenderingContext.cpp:1132 > > - m_context->getExtensions().texImage2DRobustANGLE(target, level, internalformat, width, height, border, format, type, 0, reinterpret_cast<void*>(offset)); > > + m_context->texImage2D(target, level, internalformat, width, height, border, format, type, offset); > > Was this intentional? Does texImage2D call the robust ANGLE form itself? Yes. Sorry for not describing it in the ChangeLog and not carrying the change through its logical conclusion. I uploaded a new patch, which sohuld clarify this in the ChangeLog and in the code. In the code, I moved all the texImage functions from the ExtensionsGL to GraphicsContextGL.

Kimmo Kinnunen

Comment 11 2020-11-26 05:26:00 PST

Created attachment 414896 [details] Patch

Kimmo Kinnunen

Comment 12 2020-11-26 05:38:47 PST

Created attachment 414897 [details] Patch

Kimmo Kinnunen

Comment 13 2020-11-26 05:43:53 PST

Created attachment 414898 [details] Patch

Kimmo Kinnunen

Comment 14 2020-11-26 05:50:28 PST

Created attachment 414899 [details] Patch

EWS

Comment 15 2020-11-27 01:52:03 PST

Committed r270185: <https://trac.webkit.org/changeset/270185> All reviewed patches have been landed. Closing bug and clearing flags on attachment 414899 [details].

Radar WebKit Bug Importer

Comment 16 2020-11-27 01:53:16 PST

<rdar://problem/71761224>

Philippe Normand

Comment 17 2020-11-27 03:17:02 PST

Comment on attachment 414899 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=414899&action=review > Source/WebCore/platform/graphics/GraphicsTypesGL.h:72 > + GCGLSpan(const GCGLSpan<U, Extent>& other, std::enable_if_t<std::is_convertible_v<U(*)[], T(*)[]>, nullptr_t> = nullptr) Source/WebCore/platform/graphics/GraphicsTypesGL.h:72:104: error: unknown type name 'nullptr_t'; did you mean 'std::nullptr_t'? GCGLSpan(const GCGLSpan<U, Extent>& other, std::enable_if_t<std::is_convertible_v<U(*)[], T(*)[]>, nullptr_t> = nullptr) ^~~~~~~~~ std::nullptr_t /usr/bin/../lib/gcc/x86_64-unknown-linux-gnu/10.2.0/../../../../include/c++/10.2.0/x86_64-unknown-linux-gnu/bits/c++config.h:264:29: note: 'std::nullptr_t' declared here typedef decltype(nullptr) nullptr_t; ^

Philippe Normand

Comment 18 2020-11-27 03:22:06 PST

r270189

Kimmo Kinnunen

Comment 19 2020-11-29 23:59:23 PST

Thanks Philippe

Note You need to log in before you can comment on or make changes to this bug.