From acbc3518d5453668b3b0ec08c6ca494f68977f8c Mon Sep 17 00:00:00 2001 From: sotaro Date: Tue, 28 May 2019 22:03:54 +0000 Subject: [PATCH] Bug 1554091 - Remove WrExternalImageBufferType r=jrmuizel If ExternalImageType is just passed from C to rust, it caused crash on non-Windows platform. It was caused by stack corruption. Then &ExternalImageType is used instead of ExternalImageType to bypass the problem. Differential Revision: https://phabricator.services.mozilla.com/D32436 --HG-- extra : moz-landing-system : lando --- gfx/layers/composite/TextureHost.cpp | 10 ++--- gfx/layers/d3d11/TextureD3D11.cpp | 21 +++++----- .../opengl/MacIOSurfaceTextureHostOGL.cpp | 11 +++--- gfx/layers/opengl/TextureHostOGL.cpp | 10 +++-- gfx/layers/wr/WebRenderBridgeParent.cpp | 5 +-- gfx/webrender_bindings/WebRenderAPI.cpp | 30 ++++++++------- gfx/webrender_bindings/WebRenderAPI.h | 14 ++++--- gfx/webrender_bindings/src/bindings.rs | 38 +++---------------- gfx/wr/webrender_api/src/image.rs | 1 + 9 files changed, 63 insertions(+), 77 deletions(-) diff --git a/gfx/layers/composite/TextureHost.cpp b/gfx/layers/composite/TextureHost.cpp index ec50de2ec4ae..b36071d96329 100644 --- a/gfx/layers/composite/TextureHost.cpp +++ b/gfx/layers/composite/TextureHost.cpp @@ -574,7 +574,7 @@ void BufferTextureHost::PushResourceUpdates( auto method = aOp == TextureHost::ADD_IMAGE ? &wr::TransactionBuilder::AddExternalImage : &wr::TransactionBuilder::UpdateExternalImage; - auto bufferType = wr::WrExternalImageBufferType::ExternalBuffer; + auto imageType = wr::ExternalImageType::Buffer(); if (GetFormat() != gfx::SurfaceFormat::YUV) { MOZ_ASSERT(aImageKeys.length() == 1); @@ -583,7 +583,7 @@ void BufferTextureHost::PushResourceUpdates( GetSize(), ImageDataSerializer::ComputeRGBStride(GetFormat(), GetSize().width), GetFormat()); - (aResources.*method)(aImageKeys[0], descriptor, aExtID, bufferType, 0); + (aResources.*method)(aImageKeys[0], descriptor, aExtID, imageType, 0); } else { MOZ_ASSERT(aImageKeys.length() == 3); @@ -594,9 +594,9 @@ void BufferTextureHost::PushResourceUpdates( wr::ImageDescriptor cbcrDescriptor( desc.cbCrSize(), desc.cbCrStride(), SurfaceFormatForColorDepth(desc.colorDepth())); - (aResources.*method)(aImageKeys[0], yDescriptor, aExtID, bufferType, 0); - (aResources.*method)(aImageKeys[1], cbcrDescriptor, aExtID, bufferType, 1); - (aResources.*method)(aImageKeys[2], cbcrDescriptor, aExtID, bufferType, 2); + (aResources.*method)(aImageKeys[0], yDescriptor, aExtID, imageType, 0); + (aResources.*method)(aImageKeys[1], cbcrDescriptor, aExtID, imageType, 1); + (aResources.*method)(aImageKeys[2], cbcrDescriptor, aExtID, imageType, 2); } } diff --git a/gfx/layers/d3d11/TextureD3D11.cpp b/gfx/layers/d3d11/TextureD3D11.cpp index c75f4e5aba18..528ca137108e 100644 --- a/gfx/layers/d3d11/TextureD3D11.cpp +++ b/gfx/layers/d3d11/TextureD3D11.cpp @@ -997,8 +997,9 @@ void DXGITextureHostD3D11::PushResourceUpdates( MOZ_ASSERT(aImageKeys.length() == 1); wr::ImageDescriptor descriptor(mSize, GetFormat()); - auto bufferType = wr::WrExternalImageBufferType::TextureExternalHandle; - (aResources.*method)(aImageKeys[0], descriptor, aExtID, bufferType, 0); + auto imageType = + wr::ExternalImageType::TextureHandle(wr::TextureTarget::External); + (aResources.*method)(aImageKeys[0], descriptor, aExtID, imageType, 0); break; } case gfx::SurfaceFormat::P010: @@ -1015,9 +1016,10 @@ void DXGITextureHostD3D11::PushResourceUpdates( mFormat == gfx::SurfaceFormat::NV12 ? gfx::SurfaceFormat::R8G8 : gfx::SurfaceFormat::R16G16); - auto bufferType = wr::WrExternalImageBufferType::TextureExternalHandle; - (aResources.*method)(aImageKeys[0], descriptor0, aExtID, bufferType, 0); - (aResources.*method)(aImageKeys[1], descriptor1, aExtID, bufferType, 1); + auto imageType = + wr::ExternalImageType::TextureHandle(wr::TextureTarget::External); + (aResources.*method)(aImageKeys[0], descriptor0, aExtID, imageType, 0); + (aResources.*method)(aImageKeys[1], descriptor1, aExtID, imageType, 1); break; } default: { @@ -1239,15 +1241,16 @@ void DXGIYCbCrTextureHostD3D11::PushResourceUpdates( auto method = aOp == TextureHost::ADD_IMAGE ? &wr::TransactionBuilder::AddExternalImage : &wr::TransactionBuilder::UpdateExternalImage; - auto bufferType = wr::WrExternalImageBufferType::TextureExternalHandle; + auto imageType = + wr::ExternalImageType::TextureHandle(wr::TextureTarget::External); // y wr::ImageDescriptor descriptor0(mSize, gfx::SurfaceFormat::A8); // cb and cr wr::ImageDescriptor descriptor1(mSizeCbCr, gfx::SurfaceFormat::A8); - (aResources.*method)(aImageKeys[0], descriptor0, aExtID, bufferType, 0); - (aResources.*method)(aImageKeys[1], descriptor1, aExtID, bufferType, 1); - (aResources.*method)(aImageKeys[2], descriptor1, aExtID, bufferType, 2); + (aResources.*method)(aImageKeys[0], descriptor0, aExtID, imageType, 0); + (aResources.*method)(aImageKeys[1], descriptor1, aExtID, imageType, 1); + (aResources.*method)(aImageKeys[2], descriptor1, aExtID, imageType, 2); } void DXGIYCbCrTextureHostD3D11::PushDisplayItems( diff --git a/gfx/layers/opengl/MacIOSurfaceTextureHostOGL.cpp b/gfx/layers/opengl/MacIOSurfaceTextureHostOGL.cpp index 5f7cf6890e91..590501420e93 100644 --- a/gfx/layers/opengl/MacIOSurfaceTextureHostOGL.cpp +++ b/gfx/layers/opengl/MacIOSurfaceTextureHostOGL.cpp @@ -158,7 +158,8 @@ void MacIOSurfaceTextureHostOGL::PushResourceUpdates( auto method = aOp == TextureHost::ADD_IMAGE ? &wr::TransactionBuilder::AddExternalImage : &wr::TransactionBuilder::UpdateExternalImage; - auto bufferType = wr::WrExternalImageBufferType::TextureRectHandle; + auto imageType = + wr::ExternalImageType::TextureHandle(wr::TextureTarget::Rect); switch (GetFormat()) { case gfx::SurfaceFormat::R8G8B8X8: @@ -171,7 +172,7 @@ void MacIOSurfaceTextureHostOGL::PushResourceUpdates( ? gfx::SurfaceFormat::B8G8R8A8 : gfx::SurfaceFormat::B8G8R8X8; wr::ImageDescriptor descriptor(GetSize(), format); - (aResources.*method)(aImageKeys[0], descriptor, aExtID, bufferType, 0); + (aResources.*method)(aImageKeys[0], descriptor, aExtID, imageType, 0); break; } case gfx::SurfaceFormat::YUV422: { @@ -182,7 +183,7 @@ void MacIOSurfaceTextureHostOGL::PushResourceUpdates( MOZ_ASSERT(aImageKeys.length() == 1); MOZ_ASSERT(mSurface->GetPlaneCount() == 0); wr::ImageDescriptor descriptor(GetSize(), gfx::SurfaceFormat::B8G8R8X8); - (aResources.*method)(aImageKeys[0], descriptor, aExtID, bufferType, 0); + (aResources.*method)(aImageKeys[0], descriptor, aExtID, imageType, 0); break; } case gfx::SurfaceFormat::NV12: { @@ -196,8 +197,8 @@ void MacIOSurfaceTextureHostOGL::PushResourceUpdates( gfx::IntSize(mSurface->GetDevicePixelWidth(1), mSurface->GetDevicePixelHeight(1)), gfx::SurfaceFormat::R8G8); - (aResources.*method)(aImageKeys[0], descriptor0, aExtID, bufferType, 0); - (aResources.*method)(aImageKeys[1], descriptor1, aExtID, bufferType, 1); + (aResources.*method)(aImageKeys[0], descriptor0, aExtID, imageType, 0); + (aResources.*method)(aImageKeys[1], descriptor1, aExtID, imageType, 1); break; } default: { diff --git a/gfx/layers/opengl/TextureHostOGL.cpp b/gfx/layers/opengl/TextureHostOGL.cpp index 2cf375fd4720..a7b34a02201a 100644 --- a/gfx/layers/opengl/TextureHostOGL.cpp +++ b/gfx/layers/opengl/TextureHostOGL.cpp @@ -639,7 +639,8 @@ void SurfaceTextureHost::PushResourceUpdates( auto method = aOp == TextureHost::ADD_IMAGE ? &wr::TransactionBuilder::AddExternalImage : &wr::TransactionBuilder::UpdateExternalImage; - auto bufferType = wr::WrExternalImageBufferType::TextureExternalHandle; + auto imageType = + wr::ExternalImageType::TextureHandle(wr::TextureTarget::External); switch (GetFormat()) { case gfx::SurfaceFormat::R8G8B8X8: @@ -652,7 +653,7 @@ void SurfaceTextureHost::PushResourceUpdates( ? gfx::SurfaceFormat::B8G8R8A8 : gfx::SurfaceFormat::B8G8R8X8; wr::ImageDescriptor descriptor(GetSize(), format); - (aResources.*method)(aImageKeys[0], descriptor, aExtID, bufferType, 0); + (aResources.*method)(aImageKeys[0], descriptor, aExtID, imageType, 0); break; } default: { @@ -836,7 +837,8 @@ void EGLImageTextureHost::PushResourceUpdates( auto method = aOp == TextureHost::ADD_IMAGE ? &wr::TransactionBuilder::AddExternalImage : &wr::TransactionBuilder::UpdateExternalImage; - auto bufferType = wr::WrExternalImageBufferType::TextureExternalHandle; + auto imageType = + wr::ExternalImageType::TextureHandle(wr::TextureTarget::External); gfx::SurfaceFormat format = mHasAlpha ? gfx::SurfaceFormat::R8G8B8A8 : gfx::SurfaceFormat::R8G8B8X8; @@ -848,7 +850,7 @@ void EGLImageTextureHost::PushResourceUpdates( ? gfx::SurfaceFormat::B8G8R8A8 : gfx::SurfaceFormat::B8G8R8X8; wr::ImageDescriptor descriptor(GetSize(), formatTmp); - (aResources.*method)(aImageKeys[0], descriptor, aExtID, bufferType, 0); + (aResources.*method)(aImageKeys[0], descriptor, aExtID, imageType, 0); } void EGLImageTextureHost::PushDisplayItems( diff --git a/gfx/layers/wr/WebRenderBridgeParent.cpp b/gfx/layers/wr/WebRenderBridgeParent.cpp index b89bbfb1bedf..d6ccb2ff18cb 100644 --- a/gfx/layers/wr/WebRenderBridgeParent.cpp +++ b/gfx/layers/wr/WebRenderBridgeParent.cpp @@ -574,8 +574,7 @@ bool WebRenderBridgeParent::AddExternalImage( wr::ImageDescriptor descriptor(dSurf->GetSize(), dSurf->Stride(), dSurf->GetFormat()); aResources.AddExternalImage(aKey, descriptor, aExtId, - wr::WrExternalImageBufferType::ExternalBuffer, - 0); + wr::ExternalImageType::Buffer(), 0); return true; } @@ -705,7 +704,7 @@ bool WebRenderBridgeParent::UpdateExternalImage( wr::ImageDescriptor descriptor(dSurf->GetSize(), dSurf->Stride(), dSurf->GetFormat()); aResources.UpdateExternalImageWithDirtyRect( - aKey, descriptor, aExtId, wr::WrExternalImageBufferType::ExternalBuffer, + aKey, descriptor, aExtId, wr::ExternalImageType::Buffer(), wr::ToDeviceIntRect(aDirtyRect), 0); return true; } diff --git a/gfx/webrender_bindings/WebRenderAPI.cpp b/gfx/webrender_bindings/WebRenderAPI.cpp index 7cd0d7f10cd5..fa0e07f321dc 100644 --- a/gfx/webrender_bindings/WebRenderAPI.cpp +++ b/gfx/webrender_bindings/WebRenderAPI.cpp @@ -597,19 +597,21 @@ void TransactionBuilder::AddBlobImage(BlobImageKey key, wr_resource_updates_add_blob_image(mTxn, key, &aDescriptor, &aBytes.inner); } -void TransactionBuilder::AddExternalImage( - ImageKey key, const ImageDescriptor& aDescriptor, ExternalImageId aExtID, - wr::WrExternalImageBufferType aBufferType, uint8_t aChannelIndex) { +void TransactionBuilder::AddExternalImage(ImageKey key, + const ImageDescriptor& aDescriptor, + ExternalImageId aExtID, + wr::ExternalImageType aImageType, + uint8_t aChannelIndex) { wr_resource_updates_add_external_image(mTxn, key, &aDescriptor, aExtID, - aBufferType, aChannelIndex); + &aImageType, aChannelIndex); } void TransactionBuilder::AddExternalImageBuffer( ImageKey aKey, const ImageDescriptor& aDescriptor, ExternalImageId aHandle) { auto channelIndex = 0; - AddExternalImage(aKey, aDescriptor, aHandle, - wr::WrExternalImageBufferType::ExternalBuffer, channelIndex); + AddExternalImage(aKey, aDescriptor, aHandle, wr::ExternalImageType::Buffer(), + channelIndex); } void TransactionBuilder::UpdateImageBuffer(ImageKey aKey, @@ -626,19 +628,21 @@ void TransactionBuilder::UpdateBlobImage(BlobImageKey aKey, aDirtyRect); } -void TransactionBuilder::UpdateExternalImage( - ImageKey aKey, const ImageDescriptor& aDescriptor, ExternalImageId aExtID, - wr::WrExternalImageBufferType aBufferType, uint8_t aChannelIndex) { +void TransactionBuilder::UpdateExternalImage(ImageKey aKey, + const ImageDescriptor& aDescriptor, + ExternalImageId aExtID, + wr::ExternalImageType aImageType, + uint8_t aChannelIndex) { wr_resource_updates_update_external_image(mTxn, aKey, &aDescriptor, aExtID, - aBufferType, aChannelIndex); + &aImageType, aChannelIndex); } void TransactionBuilder::UpdateExternalImageWithDirtyRect( ImageKey aKey, const ImageDescriptor& aDescriptor, ExternalImageId aExtID, - wr::WrExternalImageBufferType aBufferType, - const wr::DeviceIntRect& aDirtyRect, uint8_t aChannelIndex) { + wr::ExternalImageType aImageType, const wr::DeviceIntRect& aDirtyRect, + uint8_t aChannelIndex) { wr_resource_updates_update_external_image_with_dirty_rect( - mTxn, aKey, &aDescriptor, aExtID, aBufferType, aChannelIndex, aDirtyRect); + mTxn, aKey, &aDescriptor, aExtID, &aImageType, aChannelIndex, aDirtyRect); } void TransactionBuilder::SetImageVisibleArea(BlobImageKey aKey, diff --git a/gfx/webrender_bindings/WebRenderAPI.h b/gfx/webrender_bindings/WebRenderAPI.h index 3376db594453..e14579817ca3 100644 --- a/gfx/webrender_bindings/WebRenderAPI.h +++ b/gfx/webrender_bindings/WebRenderAPI.h @@ -128,7 +128,7 @@ class TransactionBuilder final { void AddExternalImage(ImageKey key, const ImageDescriptor& aDescriptor, ExternalImageId aExtID, - wr::WrExternalImageBufferType aBufferType, + wr::ExternalImageType aImageType, uint8_t aChannelIndex = 0); void UpdateImageBuffer(wr::ImageKey aKey, const ImageDescriptor& aDescriptor, @@ -141,13 +141,15 @@ class TransactionBuilder final { void UpdateExternalImage(ImageKey aKey, const ImageDescriptor& aDescriptor, ExternalImageId aExtID, - wr::WrExternalImageBufferType aBufferType, + wr::ExternalImageType aImageType, uint8_t aChannelIndex = 0); - void UpdateExternalImageWithDirtyRect( - ImageKey aKey, const ImageDescriptor& aDescriptor, ExternalImageId aExtID, - wr::WrExternalImageBufferType aBufferType, - const wr::DeviceIntRect& aDirtyRect, uint8_t aChannelIndex = 0); + void UpdateExternalImageWithDirtyRect(ImageKey aKey, + const ImageDescriptor& aDescriptor, + ExternalImageId aExtID, + wr::ExternalImageType aImageType, + const wr::DeviceIntRect& aDirtyRect, + uint8_t aChannelIndex = 0); void SetImageVisibleArea(BlobImageKey aKey, const wr::DeviceIntRect& aArea); diff --git a/gfx/webrender_bindings/src/bindings.rs b/gfx/webrender_bindings/src/bindings.rs index 46168415f7ed..98e59a627183 100644 --- a/gfx/webrender_bindings/src/bindings.rs +++ b/gfx/webrender_bindings/src/bindings.rs @@ -63,15 +63,6 @@ pub enum AntialiasBorder { Yes, } -#[repr(C)] -pub enum WrExternalImageBufferType { - TextureHandle = 0, - TextureRectHandle = 1, - TextureArrayHandle = 2, - TextureExternalHandle = 3, - ExternalBuffer = 4, -} - /// Used to indicate if an image is opaque, or has an alpha channel. #[repr(u8)] #[derive(Copy, Clone, Debug, PartialEq, Eq)] @@ -80,23 +71,6 @@ pub enum OpacityType { HasAlphaChannel = 1, } -impl WrExternalImageBufferType { - fn to_wr(self) -> ExternalImageType { - match self { - WrExternalImageBufferType::TextureHandle => - ExternalImageType::TextureHandle(TextureTarget::Default), - WrExternalImageBufferType::TextureRectHandle => - ExternalImageType::TextureHandle(TextureTarget::Rect), - WrExternalImageBufferType::TextureArrayHandle => - ExternalImageType::TextureHandle(TextureTarget::Array), - WrExternalImageBufferType::TextureExternalHandle => - ExternalImageType::TextureHandle(TextureTarget::External), - WrExternalImageBufferType::ExternalBuffer => - ExternalImageType::Buffer, - } - } -} - /// cbindgen:field-names=[mHandle] /// cbindgen:derive-lt=true /// cbindgen:derive-lte=true @@ -1611,7 +1585,7 @@ pub extern "C" fn wr_resource_updates_add_external_image( image_key: WrImageKey, descriptor: &WrImageDescriptor, external_image_id: WrExternalImageId, - buffer_type: WrExternalImageBufferType, + image_type: &ExternalImageType, channel_index: u8 ) { txn.add_image( @@ -1621,7 +1595,7 @@ pub extern "C" fn wr_resource_updates_add_external_image( ExternalImageData { id: external_image_id.into(), channel_index: channel_index, - image_type: buffer_type.to_wr(), + image_type: *image_type, } ), None @@ -1658,7 +1632,7 @@ pub extern "C" fn wr_resource_updates_update_external_image( key: WrImageKey, descriptor: &WrImageDescriptor, external_image_id: WrExternalImageId, - image_type: WrExternalImageBufferType, + image_type: &ExternalImageType, channel_index: u8 ) { txn.update_image( @@ -1668,7 +1642,7 @@ pub extern "C" fn wr_resource_updates_update_external_image( ExternalImageData { id: external_image_id.into(), channel_index, - image_type: image_type.to_wr(), + image_type: *image_type, } ), &DirtyRect::All, @@ -1681,7 +1655,7 @@ pub extern "C" fn wr_resource_updates_update_external_image_with_dirty_rect( key: WrImageKey, descriptor: &WrImageDescriptor, external_image_id: WrExternalImageId, - image_type: WrExternalImageBufferType, + image_type: &ExternalImageType, channel_index: u8, dirty_rect: DeviceIntRect, ) { @@ -1692,7 +1666,7 @@ pub extern "C" fn wr_resource_updates_update_external_image_with_dirty_rect( ExternalImageData { id: external_image_id.into(), channel_index, - image_type: image_type.to_wr(), + image_type: *image_type, } ), &DirtyRect::Partial(dirty_rect) diff --git a/gfx/wr/webrender_api/src/image.rs b/gfx/wr/webrender_api/src/image.rs index 829e54ebae76..1d24f18b232d 100644 --- a/gfx/wr/webrender_api/src/image.rs +++ b/gfx/wr/webrender_api/src/image.rs @@ -51,6 +51,7 @@ impl BlobImageKey { pub struct ExternalImageId(pub u64); /// Specifies the type of texture target in driver terms. +#[repr(u8)] #[derive(Copy, Clone, Debug, Eq, Hash, PartialEq, Serialize, Deserialize)] pub enum TextureTarget { /// Standard texture. This maps to GL_TEXTURE_2D in OpenGL.