WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
219387
GPU Process: Invalid static_cast from ConcreteImageBuffer to RemoteImageBufferProxy
https://bugs.webkit.org/show_bug.cgi?id=219387
Summary
GPU Process: Invalid static_cast from ConcreteImageBuffer to RemoteImageBuffe...
Tim Horton
Reported
2020-11-30 23:51:09 PST
As the subject says. This is split out of
https://bugs.webkit.org/show_bug.cgi?id=219368
Attachments
Patch
(10.26 KB, patch)
2020-11-30 23:52 PST
,
Tim Horton
no flags
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Tim Horton
Comment 1
2020-11-30 23:52:06 PST
Created
attachment 415102
[details]
Patch
EWS
Comment 2
2020-12-01 00:25:03 PST
Committed
r270286
: <
https://trac.webkit.org/changeset/270286
> All reviewed patches have been landed. Closing bug and clearing flags on
attachment 415102
[details]
.
Radar WebKit Bug Importer
Comment 3
2020-12-01 00:26:16 PST
<
rdar://problem/71840749
>
Said Abou-Hallawa
Comment 4
2020-12-01 01:01:56 PST
Comment on
attachment 415102
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=415102&action=review
> Source/WebKit/Shared/ConcreteShareableImageBuffer.h:34 > +class ConcreteShareableImageBuffer : public WebCore::ConcreteImageBuffer<BackendType> {
If you expose the backend from ImageBuffer (see below), I think you will not need this class.
> Source/WebKit/Shared/ConcreteShareableImageBuffer.h:39 > + static auto create(const WebCore::FloatSize& size, WebCore::RenderingMode renderingMode, float resolutionScale, WebCore::ColorSpace colorSpace, WebCore::PixelFormat pixelFormat)
Unused argument renderingMode.
> Source/WebKit/Shared/RemoteLayerTree/RemoteLayerBackingStore.mm:119 > + if (WebProcess::singleton().shouldUseRemoteRenderingFor(WebCore::RenderingPurpose::DOM)) { > + if (m_acceleratesDrawing) > + handle = static_cast<AcceleratedRemoteImageBufferProxy *>(m_frontBuffer.imageBuffer.get())->createImageBufferBackendHandle(); > + else > + handle = static_cast<UnacceleratedRemoteImageBufferProxy *>(m_frontBuffer.imageBuffer.get())->createImageBufferBackendHandle(); > + } else { > + if (m_acceleratesDrawing) > + handle = static_cast<ConcreteShareableImageBuffer<AcceleratedImageBufferShareableBackend> *>(m_frontBuffer.imageBuffer.get())->createImageBufferBackendHandle(); > + else > + handle = static_cast<ConcreteShareableImageBuffer<UnacceleratedImageBufferShareableBackend> *>(m_frontBuffer.imageBuffer.get())->createImageBufferBackendHandle(); > + }
Can't this casting be cleaned by exposing the ImageBufferBackend from ImageBuffer? This code can be simplified like this: if (m_acceleratesDrawing) handle = static_cast<ImageBufferShareableIOSurfaceBackend&>(*m_frontBuffer.imageBuffer->backend()).createImageBufferBackendHandle(); else handle = static_cast<ImageBufferShareableBitmapBackend&>(*m_frontBuffer.imageBuffer->backend()).createImageBufferBackendHandle();
> Source/WebKit/Shared/RemoteLayerTree/RemoteLayerBackingStore.mm:196 > + m_frontBuffer.imageBuffer = ConcreteShareableImageBuffer<AcceleratedImageBufferShareableBackend>::create(backingStoreSize(), WebCore::RenderingMode::Accelerated, 1, WebCore::ColorSpace::SRGB, pixelFormat());
No need to pass the rendering mode to a concrete ImageBuffer since the backend type already reveals the type of the backend.
Said Abou-Hallawa
Comment 5
2020-12-01 01:08:28 PST
Comment on
attachment 415102
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=415102&action=review
>> Source/WebKit/Shared/RemoteLayerTree/RemoteLayerBackingStore.mm:196 >> + m_frontBuffer.imageBuffer = ConcreteShareableImageBuffer<AcceleratedImageBufferShareableBackend>::create(backingStoreSize(), WebCore::RenderingMode::Accelerated, 1, WebCore::ColorSpace::SRGB, pixelFormat()); > > No need to pass the rendering mode to a concrete ImageBuffer since the backend type already reveals the type of the backend.
I meant to say "... the backend type reveals the renderingMode".
Tim Horton
Comment 6
2020-12-01 01:08:47 PST
(In reply to Said Abou-Hallawa from
comment #4
)
> Comment on
attachment 415102
[details]
> Patch > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=415102&action=review
> > > Source/WebKit/Shared/ConcreteShareableImageBuffer.h:34 > > +class ConcreteShareableImageBuffer : public WebCore::ConcreteImageBuffer<BackendType> { > > If you expose the backend from ImageBuffer (see below), I think you will not > need this class. > > > Source/WebKit/Shared/ConcreteShareableImageBuffer.h:39 > > + static auto create(const WebCore::FloatSize& size, WebCore::RenderingMode renderingMode, float resolutionScale, WebCore::ColorSpace colorSpace, WebCore::PixelFormat pixelFormat) > > Unused argument renderingMode. > > > Source/WebKit/Shared/RemoteLayerTree/RemoteLayerBackingStore.mm:119 > > + if (WebProcess::singleton().shouldUseRemoteRenderingFor(WebCore::RenderingPurpose::DOM)) { > > + if (m_acceleratesDrawing) > > + handle = static_cast<AcceleratedRemoteImageBufferProxy *>(m_frontBuffer.imageBuffer.get())->createImageBufferBackendHandle(); > > + else > > + handle = static_cast<UnacceleratedRemoteImageBufferProxy *>(m_frontBuffer.imageBuffer.get())->createImageBufferBackendHandle(); > > + } else { > > + if (m_acceleratesDrawing) > > + handle = static_cast<ConcreteShareableImageBuffer<AcceleratedImageBufferShareableBackend> *>(m_frontBuffer.imageBuffer.get())->createImageBufferBackendHandle(); > > + else > > + handle = static_cast<ConcreteShareableImageBuffer<UnacceleratedImageBufferShareableBackend> *>(m_frontBuffer.imageBuffer.get())->createImageBufferBackendHandle(); > > + } > > Can't this casting be cleaned by exposing the ImageBufferBackend from > ImageBuffer? This code can be simplified like this: > > if (m_acceleratesDrawing) > handle = > static_cast<ImageBufferShareableIOSurfaceBackend&>(*m_frontBuffer. > imageBuffer->backend()).createImageBufferBackendHandle(); > else > handle = > static_cast<ImageBufferShareableBitmapBackend&>(*m_frontBuffer.imageBuffer- > >backend()).createImageBufferBackendHandle();
I am hoping to figure out how to finagle createImageBufferBackendHandle onto ImageBuffer, so we don't have /any/ bifurcation or static casts. You're right that it will obsolete most of this patch, but it's trickier, and the invalid cast seemed ... very bad.
> > Source/WebKit/Shared/RemoteLayerTree/RemoteLayerBackingStore.mm:196 > > + m_frontBuffer.imageBuffer = ConcreteShareableImageBuffer<AcceleratedImageBufferShareableBackend>::create(backingStoreSize(), WebCore::RenderingMode::Accelerated, 1, WebCore::ColorSpace::SRGB, pixelFormat()); > > No need to pass the rendering mode to a concrete ImageBuffer since the > backend type already reveals the type of the backend.
Good point.
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