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
2019-05-28 22:03:54 +00:00 · 2019-05-28 22:03:54 +00:00 · acbc3518d5
--- 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);
  }
 }

--- 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(
--- 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: {
--- 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(
--- 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;
  }
--- 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,
--- 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);

--- 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)
--- 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.