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

Fixed interaction between dynamic texture and context.clip() #1185

Merged

Conversation

SergioRZMasson
Copy link
Contributor

Overview

Allows Dynamic Texture context context.drawImage context.clearRect and context.fillRect to properly work with context.clip.

Implementation

When calling Babylon::Polyfills::Internal::Context::DrawImage the nvgBeginPath is been called every time. This will discard any path that was previously set by the user. The correct behavior should be to not call nvgBeginPath if Babylon::Polyfills::Internal::Context::Clip was previously called.

Also, in Babylon::Polyfills::Internal::Context::Clip the m_rectangleClipping is been used to set the Scissor values even when m_rectangleClipping was never set before (with default values 0, 0, 0, 0). This will make some of the following operations to produce no result. The correct behavior should be to only use m_rectangleClipping if the were set, otherwise use the width and height of the render target.

Finally, when creating Babylon::Polyfills::Internal::NativeCanvasImage the resulting data might be in RGB instead of RGBA. However, during NativeCanvasImage::CreateNVGImageForContext NVG always assumes the format to be RGBA, which might cause a memory out of bounds read if the image was originally RGB. This PR forces bimg::imageParse to always convert the image to RGBA.

More information

Allows the following code to work in Babylon Native: https://playground.babylonjs.com/#FU0ES5#27
Fixes issue #1184

Apps/ValidationTests/Scripts/config.json Outdated Show resolved Hide resolved
azure-pipelines.yml Outdated Show resolved Hide resolved
Apps/ValidationTests/Scripts/config.json Outdated Show resolved Hide resolved
Apps/ValidationTests/Scripts/config.json Outdated Show resolved Hide resolved
Copy link
Contributor

@CedricGuillemet CedricGuillemet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@SergioRZMasson SergioRZMasson merged commit 202b7c6 into BabylonJS:master Jan 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants