.
Created attachment 454658 [details] Patch
Comment on attachment 454658 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=454658&action=review > Source/WebGPU/WebGPU/CommandEncoder.mm:144 > + if (validateCopyBufferToBuffer(source, sourceOffset, destination, destinationOffset, size)) { if (!validateCopyBufferToBuffer(source, sourceOffset, destination, destinationOffset, size))
Created attachment 454664 [details] Patch
Comment on attachment 454664 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=454664&action=review > Source/WebGPU/WebGPU/CommandEncoder.mm:52 > + commandBuffer.label = [NSString stringWithCString:descriptor.label encoding:NSUTF8StringEncoding]; nullptr label remark (in case it's not handled in the fromAPI patch) > Source/WebGPU/WebGPU/CommandsMixin.h:32 > +class CommandsMixin : public RefCounted<CommandsMixin> { Based on this commit alone, it's not clear why would the refcounted be in the mixin. it would indicate objects hold references to the mixin, which is counter (my) expectation of mixins being just "way to import a bag of common functionality" Also having the WTF_MAKE_FAST_ALLOCATED here would imply you allocate the mixins
Comment on attachment 454664 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=454664&action=review >> Source/WebGPU/WebGPU/CommandEncoder.mm:52 >> + commandBuffer.label = [NSString stringWithCString:descriptor.label encoding:NSUTF8StringEncoding]; > > nullptr label remark (in case it's not handled in the fromAPI patch) Checked, I think the fromAPI patch was not written with this patch in, so there's that. so probably you could have some sort of fromAPI<NSString*>(descriptor.label) for these cases (or something else you come up with)
Comment on attachment 454664 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=454664&action=review >>> Source/WebGPU/WebGPU/CommandEncoder.mm:52 >>> + commandBuffer.label = [NSString stringWithCString:descriptor.label encoding:NSUTF8StringEncoding]; >> >> nullptr label remark (in case it's not handled in the fromAPI patch) > > Checked, I think the fromAPI patch was not written with this patch in, so there's that. > so probably you could have some sort of fromAPI<NSString*>(descriptor.label) for these cases (or something else you come up with) Right. I'll have to update the fromAPI patch after this lands. >> Source/WebGPU/WebGPU/CommandsMixin.h:32 >> +class CommandsMixin : public RefCounted<CommandsMixin> { > > Based on this commit alone, it's not clear why would the refcounted be in the mixin. it would indicate objects hold references to the mixin, which is counter (my) expectation of mixins being just "way to import a bag of common functionality" > > Also having the WTF_MAKE_FAST_ALLOCATED here would imply you allocate the mixins Ah, I see what you're saying. Yeah, I guess multiple inheritance is better than making a mixin class that appears to be refcounted.
Committed r291372 (248504@trunk): <https://commits.webkit.org/248504@trunk>
<rdar://problem/90394764>