WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
238719
[WebGPU] Add support for the "is valid to use with" operation to WebGPU objects
https://bugs.webkit.org/show_bug.cgi?id=238719
Summary
[WebGPU] Add support for the "is valid to use with" operation to WebGPU objects
Myles C. Maxfield
Reported
2022-04-03 23:38:42 PDT
.
Attachments
Patch
(37.66 KB, patch)
2022-04-03 23:44 PDT
,
Myles C. Maxfield
no flags
Details
Formatted Diff
Diff
Patch
(56.92 KB, patch)
2022-04-04 01:05 PDT
,
Myles C. Maxfield
kkinnunen
: review+
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Myles C. Maxfield
Comment 1
2022-04-03 23:44:20 PDT
Created
attachment 456535
[details]
Patch
Myles C. Maxfield
Comment 2
2022-04-04 01:05:24 PDT
Created
attachment 456546
[details]
Patch
Kimmo Kinnunen
Comment 3
2022-04-07 02:57:39 PDT
Comment on
attachment 456546
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=456546&action=review
also changes refcounts to thread safe (which is probably good)
> Source/WebGPU/WebGPU/ObjectBase.h:40 > + ~ObjectBase();
Surprisingly ThreadSafeRefCounted has a footgun where the destructor is not a virtual. So written this way, Ref<ObjectBase> would never run the subclass destructors, which is a hard to find down the line if you ever end up using that. It's better to mark this virtual. If you don't intend to support Ref<ObjectBase>, then it's better to leave the threadsaferefcounted in the held classes.
Myles C. Maxfield
Comment 4
2022-04-07 12:18:31 PDT
Comment on
attachment 456546
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=456546&action=review
>> Source/WebGPU/WebGPU/ObjectBase.h:40 >> + ~ObjectBase(); > > Surprisingly ThreadSafeRefCounted has a footgun where the destructor is not a virtual. > So written this way, Ref<ObjectBase> would never run the subclass destructors, which is a hard to find down the line if you ever end up using that. > It's better to mark this virtual. > > If you don't intend to support Ref<ObjectBase>, then it's better to leave the threadsaferefcounted in the held classes.
Thinking about this more, I think there may actually be a better way of doing this. The whole point of this ObjectBase class is just to hold the isValidToUseWith() function, which is already templated. So, if it's already templated, it doesn't really need to actually exist in the base class. It can just be a free function and call .device() on the templated type. Doing it that way will get rid of this whole type hierarchy, and the problem you're describing here, as well as reducing total amount of code.
Myles C. Maxfield
Comment 5
2022-04-07 13:23:16 PDT
Committed
r292558
(
249396@trunk
): <
https://commits.webkit.org/249396@trunk
>
Radar WebKit Bug Importer
Comment 6
2022-04-07 13:24:24 PDT
<
rdar://problem/91440590
>
Myles C. Maxfield
Comment 7
2022-04-07 15:50:28 PDT
Committed
r292574
(?): <
https://commits.webkit.org/r292574
>
Dan Glastonbury
Comment 8
2022-04-07 18:58:30 PDT
(In reply to Myles C. Maxfield from
comment #4
)
> [...] It > can just be a free function and call .device() on the templated type. Doing > it that way will get rid of this whole type hierarchy, and the problem > you're describing here, as well as reducing total amount of code.
I like the sound of that!
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