Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ImageBufferBackend::getPixelBuffer has redundant allocator parameter #13870

Conversation

kkinnunen-apple
Copy link
Contributor

@kkinnunen-apple kkinnunen-apple commented May 15, 2023

61e0dd7

ImageBufferBackend::getPixelBuffer has redundant allocator parameter
https://bugs.webkit.org/show_bug.cgi?id=256778
rdar://109343550

Reviewed by Said Abou-Hallawa.

ImageBufferBackend::getPixelBuffer is a virtual function where
subclasses can customize the behavior. The PixelBuffer allocator
parameter is not used in polymorphic way. Instead, the caller of
ImageBufferBackend::getImageBuffer, e.g. ImageBuffer and
RemoteImageBufferProxy, can create the PixelBuffer before invoking
getPixelBuffer.

The removal simplifying the getImageBuffer API is needed so that adding
actually useful parameters does not cause overly complicated API.

Makes the rect and point arguments of
ImageBufferBackend::getPixelBuffer/putPixelBuffer be in backend
coordinate system instead of "logical size" coordinate system. The
"logical size" doesn't make sense for the backend, rather backend is
always in raw pixel coordinates.

Fixes undefined behavior wrt read before write in constructs such as:
  ConstPixelBufferConversionView source {
      ...,
      [bytesPerRow initializer],
      ..., x + source.bytesPerRow * y, ...
  };

* Source/WebCore/platform/graphics/ImageBuffer.cpp:
(WebCore::ImageBuffer::getPixelBuffer const):
(WebCore::ImageBuffer::putPixelBuffer):
* Source/WebCore/platform/graphics/ImageBufferBackend.cpp:
(WebCore::ImageBufferBackend::convertToLuminanceMask):
(WebCore::ImageBufferBackend::getPixelBuffer):
(WebCore::ImageBufferBackend::putPixelBuffer):
* Source/WebCore/platform/graphics/ImageBufferBackend.h:
* Source/WebCore/platform/graphics/cairo/ImageBufferCairoSurfaceBackend.cpp:
(WebCore::ImageBufferCairoSurfaceBackend::getPixelBuffer):
* Source/WebCore/platform/graphics/cairo/ImageBufferCairoSurfaceBackend.h:
* Source/WebCore/platform/graphics/cg/ImageBufferCGBitmapBackend.cpp:
(WebCore::ImageBufferCGBitmapBackend::getPixelBuffer):
* Source/WebCore/platform/graphics/cg/ImageBufferCGBitmapBackend.h:
* Source/WebCore/platform/graphics/cg/ImageBufferIOSurfaceBackend.cpp:
(WebCore::ImageBufferIOSurfaceBackend::getPixelBuffer):
* Source/WebCore/platform/graphics/cg/ImageBufferIOSurfaceBackend.h:
* Source/WebKit/Shared/RemoteLayerTree/CGDisplayListImageBufferBackend.h:
* Source/WebKit/Shared/RemoteLayerTree/CGDisplayListImageBufferBackend.mm:
(WebKit::CGDisplayListImageBufferBackend::getPixelBuffer):
* Source/WebKit/WebProcess/GPU/graphics/ImageBufferShareableBitmapBackend.cpp:
(WebKit::ImageBufferShareableBitmapBackend::getPixelBuffer):
* Source/WebKit/WebProcess/GPU/graphics/ImageBufferShareableBitmapBackend.h:
* Source/WebKit/WebProcess/GPU/graphics/cocoa/ImageBufferRemoteIOSurfaceBackend.cpp:
(WebKit::ImageBufferRemoteIOSurfaceBackend::getPixelBuffer):
* Source/WebKit/WebProcess/GPU/graphics/cocoa/ImageBufferRemoteIOSurfaceBackend.h:
* Source/WebKit/WebProcess/GPU/graphics/cocoa/ImageBufferShareableMappedIOSurfaceBitmapBackend.cpp:
(WebKit::ImageBufferShareableMappedIOSurfaceBitmapBackend::getPixelBuffer):
* Source/WebKit/WebProcess/GPU/graphics/cocoa/ImageBufferShareableMappedIOSurfaceBitmapBackend.h:

Canonical link: https://commits.webkit.org/264239@main

ab3403e

Misc iOS, tvOS & watchOS macOS Linux Windows
βœ… πŸ§ͺ style βœ… πŸ›  ios βœ… πŸ›  mac βœ… πŸ›  wpe βœ… πŸ›  wincairo
βœ… πŸ§ͺ bindings βœ… πŸ›  ios-sim βœ… πŸ›  mac-AS-debug βœ… πŸ§ͺ wpe-wk2
βœ… πŸ§ͺ webkitperl βœ… πŸ§ͺ ios-wk2 βœ… πŸ§ͺ api-mac βœ… πŸ›  gtk
βœ… πŸ§ͺ ios-wk2-wpt βœ… πŸ§ͺ mac-wk1 ❌ πŸ§ͺ gtk-wk2
βœ… πŸ§ͺ api-ios βœ… πŸ§ͺ mac-wk2 βœ… πŸ§ͺ api-gtk
βœ… πŸ›  tv βœ… πŸ§ͺ mac-AS-debug-wk2
βœ… πŸ›  tv-sim
βœ… πŸ›  πŸ§ͺ merge βœ… πŸ›  watch
βœ… πŸ›  watch-sim

@kkinnunen-apple kkinnunen-apple self-assigned this May 15, 2023
@kkinnunen-apple kkinnunen-apple added the Canvas Bugs related to the canvas element. label May 15, 2023
@kkinnunen-apple kkinnunen-apple requested review from shallawa and removed request for cdumez May 15, 2023 10:50
Source/WebCore/platform/graphics/ImageBuffer.cpp Outdated Show resolved Hide resolved
Source/WebCore/platform/graphics/ImageBuffer.cpp Outdated Show resolved Hide resolved
@kkinnunen-apple kkinnunen-apple force-pushed the imagebuffer-context-did-draw-to-imagebuffer-1 branch from 7afb92a to ab3403e Compare May 17, 2023 08:01
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label May 17, 2023
@kkinnunen-apple kkinnunen-apple added merge-queue Applied to send a pull request to merge-queue and removed merging-blocked Applied to prevent a change from being merged labels May 19, 2023
https://bugs.webkit.org/show_bug.cgi?id=256778
rdar://109343550

Reviewed by Said Abou-Hallawa.

ImageBufferBackend::getPixelBuffer is a virtual function where
subclasses can customize the behavior. The PixelBuffer allocator
parameter is not used in polymorphic way. Instead, the caller of
ImageBufferBackend::getImageBuffer, e.g. ImageBuffer and
RemoteImageBufferProxy, can create the PixelBuffer before invoking
getPixelBuffer.

The removal simplifying the getImageBuffer API is needed so that adding
actually useful parameters does not cause overly complicated API.

Makes the rect and point arguments of
ImageBufferBackend::getPixelBuffer/putPixelBuffer be in backend
coordinate system instead of "logical size" coordinate system. The
"logical size" doesn't make sense for the backend, rather backend is
always in raw pixel coordinates.

Fixes undefined behavior wrt read before write in constructs such as:
  ConstPixelBufferConversionView source {
      ...,
      [bytesPerRow initializer],
      ..., x + source.bytesPerRow * y, ...
  };

* Source/WebCore/platform/graphics/ImageBuffer.cpp:
(WebCore::ImageBuffer::getPixelBuffer const):
(WebCore::ImageBuffer::putPixelBuffer):
* Source/WebCore/platform/graphics/ImageBufferBackend.cpp:
(WebCore::ImageBufferBackend::convertToLuminanceMask):
(WebCore::ImageBufferBackend::getPixelBuffer):
(WebCore::ImageBufferBackend::putPixelBuffer):
* Source/WebCore/platform/graphics/ImageBufferBackend.h:
* Source/WebCore/platform/graphics/cairo/ImageBufferCairoSurfaceBackend.cpp:
(WebCore::ImageBufferCairoSurfaceBackend::getPixelBuffer):
* Source/WebCore/platform/graphics/cairo/ImageBufferCairoSurfaceBackend.h:
* Source/WebCore/platform/graphics/cg/ImageBufferCGBitmapBackend.cpp:
(WebCore::ImageBufferCGBitmapBackend::getPixelBuffer):
* Source/WebCore/platform/graphics/cg/ImageBufferCGBitmapBackend.h:
* Source/WebCore/platform/graphics/cg/ImageBufferIOSurfaceBackend.cpp:
(WebCore::ImageBufferIOSurfaceBackend::getPixelBuffer):
* Source/WebCore/platform/graphics/cg/ImageBufferIOSurfaceBackend.h:
* Source/WebKit/Shared/RemoteLayerTree/CGDisplayListImageBufferBackend.h:
* Source/WebKit/Shared/RemoteLayerTree/CGDisplayListImageBufferBackend.mm:
(WebKit::CGDisplayListImageBufferBackend::getPixelBuffer):
* Source/WebKit/WebProcess/GPU/graphics/ImageBufferShareableBitmapBackend.cpp:
(WebKit::ImageBufferShareableBitmapBackend::getPixelBuffer):
* Source/WebKit/WebProcess/GPU/graphics/ImageBufferShareableBitmapBackend.h:
* Source/WebKit/WebProcess/GPU/graphics/cocoa/ImageBufferRemoteIOSurfaceBackend.cpp:
(WebKit::ImageBufferRemoteIOSurfaceBackend::getPixelBuffer):
* Source/WebKit/WebProcess/GPU/graphics/cocoa/ImageBufferRemoteIOSurfaceBackend.h:
* Source/WebKit/WebProcess/GPU/graphics/cocoa/ImageBufferShareableMappedIOSurfaceBitmapBackend.cpp:
(WebKit::ImageBufferShareableMappedIOSurfaceBitmapBackend::getPixelBuffer):
* Source/WebKit/WebProcess/GPU/graphics/cocoa/ImageBufferShareableMappedIOSurfaceBitmapBackend.h:

Canonical link: https://commits.webkit.org/264239@main
@webkit-commit-queue webkit-commit-queue force-pushed the imagebuffer-context-did-draw-to-imagebuffer-1 branch from ab3403e to 61e0dd7 Compare May 19, 2023 08:35
@webkit-commit-queue
Copy link
Collaborator

Committed 264239@main (61e0dd7): https://commits.webkit.org/264239@main

Reviewed commits have been landed. Closing PR #13870 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit 61e0dd7 into WebKit:main May 19, 2023
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label May 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Canvas Bugs related to the canvas element.
Projects
None yet
5 participants