Factor out pixel buffer from DOM specific ImageData class
Created attachment 428075 [details] Patch
Created attachment 428086 [details] Patch
Created attachment 428087 [details] Patch
Created attachment 428090 [details] Patch
Comment on attachment 428090 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=428090&action=review > Source/WebCore/bindings/js/SerializedScriptValue.cpp:1085 > + RefPtr<ArrayBuffer> arrayBuffer = imageData->data().possiblySharedBuffer(); We could either do auto/makeRefPtr, or just say RefPtr instead of RefPtr<ArrayBuffer>, adding deduction guides to RefPtr if needed. The future of RefPtr is now! > Source/WebCore/html/ImageData.h:56 > + ImageData(PixelBuffer&&); explicit > Source/WebCore/platform/graphics/PixelBuffer.h:31 > +#include <JavaScriptCore/Uint8ClampedArray.h> Do we really need to include this header? Can we do a forward-declaration instead? > Source/WebCore/platform/graphics/PixelBuffer.h:39 > + ~PixelBuffer(); If the only thing this does is decrement the Uint8ClampedArray reference count, then maybe we can just use the default and not write this at all. > Source/WebCore/platform/graphics/ShadowBlur.cpp:912 > + auto& blurPixelArray = layerData->data(); > + blurLayerImage(blurPixelArray.data(), blurRect.size(), blurRect.width() * 4); Seems like we don’t need the local at all. > Source/WebCore/platform/graphics/filters/FELighting.cpp:482 > + auto& resutPixelArray = resultImage->data(); Existing typo, "resut", that you should fix. > Source/WebCore/platform/graphics/filters/FEMorphology.cpp:250 > + auto& dstPixelArray = resultImage->data(); Do we have to do the "dst" thing? Maybe write out the whole destination word. > Source/WebCore/platform/graphics/filters/FilterEffect.cpp:204 > + Uint8ClampedArray& imageArray = m_premultipliedImageResult->data(); auto&
(In reply to Darin Adler from comment #5) > Comment on attachment 428090 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=428090&action=review > > > Source/WebCore/bindings/js/SerializedScriptValue.cpp:1085 > > + RefPtr<ArrayBuffer> arrayBuffer = imageData->data().possiblySharedBuffer(); > > We could either do auto/makeRefPtr, or just say RefPtr instead of > RefPtr<ArrayBuffer>, adding deduction guides to RefPtr if needed. The future > of RefPtr is now! I went with auto/makeRefPtr, but I ashamed. `RefPtr arrayBuffer = foo();` looks just too weird for me :). I'll get used to it though. > > > Source/WebCore/html/ImageData.h:56 > > + ImageData(PixelBuffer&&); > > explicit Done. > > > Source/WebCore/platform/graphics/PixelBuffer.h:31 > > +#include <JavaScriptCore/Uint8ClampedArray.h> > > Do we really need to include this header? Can we do a forward-declaration > instead? The forward declaration for these types is non-trivial to say the least. Something like: namespace JSC { struct Uint8ClampedAdaptor; template<typename Adaptor> class GenericTypedArrayView; typedef GenericTypedArrayView<Uint8ClampedAdaptor> Uint8ClampedArray; } but getting that building everywhere is quite a challenge. I think PixelBuffer will evolve to use ArrayBuffer directly at some point, at which point it should be a bit easier and I will attempt it then. > > > Source/WebCore/platform/graphics/PixelBuffer.h:39 > > + ~PixelBuffer(); > > If the only thing this does is decrement the Uint8ClampedArray reference > count, then maybe we can just use the default and not write this at all. I wanted it out of line to avoid including all the inline header inclusion that would be required due to JSC header fun. > > > Source/WebCore/platform/graphics/ShadowBlur.cpp:912 > > + auto& blurPixelArray = layerData->data(); > > + blurLayerImage(blurPixelArray.data(), blurRect.size(), blurRect.width() * 4); > > Seems like we don’t need the local at all. Agreed. Fixed. > > > Source/WebCore/platform/graphics/filters/FELighting.cpp:482 > > + auto& resutPixelArray = resultImage->data(); > > Existing typo, "resut", that you should fix. Fixed. > > > Source/WebCore/platform/graphics/filters/FEMorphology.cpp:250 > > + auto& dstPixelArray = resultImage->data(); > > Do we have to do the "dst" thing? Maybe write out the whole destination word. Fixed. Used `destinationPixelArray`. > > > Source/WebCore/platform/graphics/filters/FilterEffect.cpp:204 > > + Uint8ClampedArray& imageArray = m_premultipliedImageResult->data(); > > auto& Fixed.
Created attachment 428098 [details] Patch
Created attachment 428099 [details] Patch
Created attachment 428100 [details] Patch
Patch 428098 does not build
Committed r277237 (237506@main): <https://commits.webkit.org/237506@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 428100 [details].
<rdar://problem/77702646>