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

Fix babylon.js logic for BabylonNative Canvas polyfill #10518

Merged
merged 6 commits into from
Jun 18, 2021

Conversation

chrisfromwork
Copy link
Contributor

This review contains the following changes:

  1. We update the updateDynamicTexture definition for the nativeEngine to match the function used in BabylonNative examples
  2. We remove a document availability check for the TouchHolographicButton that is no longer needed
  3. We define a copyTexture call for _native thats been implemented in BabylonNative

@chrisfromwork chrisfromwork changed the title This change fixes items related to the BabylonNative Canvas polyfill Fix babylon.js logic for BabylonNative Canvas polyfill Jun 17, 2021
premulAlpha = false;
}

if (texture != null &&
Copy link
Member

Choose a reason for hiding this comment

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

Should be if (texture !== null && or just if (texture &&

Copy link
Member

Choose a reason for hiding this comment

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

And same for check after that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this should now be fixed

@chrisfromwork chrisfromwork requested a review from ryantrem June 17, 2021 23:53

if (DomManagement.IsDocumentAvailable() && !!document.createElement) {
Copy link
Contributor

@rickfromwork rickfromwork Jun 18, 2021

Choose a reason for hiding this comment

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

Can you remove the check around the text portion of _rebuildContent in HolographicButton as well?

Since it looks like Image still needs the check, we can ignore HolographicSlate for now. #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this should now be fixed

Copy link
Contributor

@rickfromwork rickfromwork left a comment

Choose a reason for hiding this comment

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

:shipit:

@chrisfromwork chrisfromwork merged commit aee2550 into master Jun 18, 2021
@RaananW RaananW deleted the canvasFixes branch June 21, 2021 10:23
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.

4 participants