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

chore: move alpha_texture from PBRMaterial to UnlitMaterial #2482

Merged

Conversation

AlejandroAlvarezMelucciDCL
Copy link
Collaborator

@AlejandroAlvarezMelucciDCL AlejandroAlvarezMelucciDCL commented Oct 17, 2024

WHY

Due to performance reasons the Explorer Alpha won't be supporting alpha texture on SDK PBR Materials any time soon.
However, the shader used for Unlit Material supports alpha texture.

WHAT

Addresses the E@ part of issue #240

Related PRs:
Protocol: decentraland/protocol#223
SDK: decentraland/js-sdk-toolchain#1019

QA TEST INSTRUCTIONS

  1. Download the build from this PR
  2. Go to AleAlvarez world: /goto AleAlvarez
  3. There are 2 pillars, one is to show the alpha texture for video player on the OLD client, and one is for the NEW client. Click on both to check it works on the New Client (Old one is using PBR Material, New one is using BasicMaterial (Unlit)

…al system to support the new alphaTexture property of the BaseMaterial component. Removed and ignored alpha texture request and assign for PBR material
@AlejandroAlvarezMelucciDCL AlejandroAlvarezMelucciDCL marked this pull request as ready for review October 17, 2024 23:49
@AlejandroAlvarezMelucciDCL AlejandroAlvarezMelucciDCL requested review from a team and removed request for dalkia, lorux0 and Ludmilafantaniella October 17, 2024 23:50
Copy link
Member

@pravusjif pravusjif left a comment

Choose a reason for hiding this comment

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

LGTM.

Remember to update the protocol package before merging

@Ludmilafantaniella Ludmilafantaniella self-requested a review October 18, 2024 20:09
Copy link

@Ludmilafantaniella Ludmilafantaniella left a comment

Choose a reason for hiding this comment

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

🟢 Chore verified on Mac and Window. Both pillars have the corresponding video materials (Old one is using PBR Material, New one is using BasicMaterial (Unlit). Approved by QA.

2482.mp4

Smoke test was performed covering:

  • Scenes with video streaming like Casa Roustan
  • Backpack (emotes and wereables)
  • Metadynelabs world

@pravusjif pravusjif requested a review from dalkia October 21, 2024 08:56
…re/move-alphaTexture-from-PBRMaterial-to-UnlitMaterial
@pravusjif pravusjif requested a review from GBirch33 October 21, 2024 12:27
@pravusjif pravusjif merged commit dde6404 into dev Oct 21, 2024
5 checks passed
@pravusjif pravusjif deleted the chore/move-alphaTexture-from-PBRMaterial-to-UnlitMaterial branch October 21, 2024 16:18
m3taphysics added a commit that referenced this pull request Oct 22, 2024
* refactor: Use the bucket public url (#2486)

* fix: Use the bucket public url

* fix: Branch name reference

* refactor: Use a defined github var

* add support for global sounds (#2495)

* fix: Scene Parallax Dynamic Branching fix (#2459)

* Scene Parallax Dynamic Branching fix

* Add the Mac shaders

* chore: 'Cannot connect to destination hsot' mac mitigation (#2425)

* Hacky force unload

* More force unloading

* private force unloading

* Remove dependency unload

* Remove Resources.Unload in aggresive strategy. Not useful

* Fix cacheable url

* Fixed dependency unloading

* Fix main asset unload

* Fix test

* Fixed more tests

* Fixed more tests

* more mote tests

* Revert priority

* null check in GatherGLTFAssetsSystem.cs

* Revert "Remove Resources.Unload in aggresive strategy. Not useful"

This reverts commit 383e5fe.

* refactored unload strategy

* Test coverage

* Textures leaked when leaving and entering Genesis plaza (#2129) (#2484)

* fix: Unable to recover initial memory state in GP (#2129)

Now cached texture intentions are compared using the texture file hash instead of the URL that was used to download it. Now it does not matter which content server is the candidate to get the texture from, if the instance was previously stored, it will reuse it when entering Genesis plaza.

* Cleaning code, step 1

Created a separated method to get the file hash.

* Cleaning step 2

* Cleaning step 3

* Null check (#2501)

* replaced data type in query: (#2504)

* chore: sync main to dev (#2507)

Co-authored-by: Ashley Canning <[email protected]>

* added missing IFinalizeWorldSystem implementation in system (#2506)

* Fix: UnityWebRequest exception request aborted (#2498)

## What does this PR change?
Add dispose for the request and exception not thrown after that

## How to test the changes?
not needed

* Fix: Handle RunConnectCycleStepAsync safer (#2478)

* Handle RunConnectCycleStepAsync safer

* Add logs in case of failed delivery

* More logs

* Preserve the previous connection if the players returns to the scene

* Fix connectedScene assigning

* Restore ConnectiveRoom.Fake

* fix: bind connection status panel visibility to debug command (#2441)

* fix: weekly release page (#2513)

Signed-off-by: Juan Ignacio Molteni <[email protected]>

* chore: move alpha_texture from PBRMaterial to UnlitMaterial (#2482)

Implemented support for alpha texture on SDK unlit material component

* fix: Textures in the panels at Genesis plaza main spawn area are not appearing (#2514)

Those panels use textures downloaded from arbitrary URLs and not from the content servers, which means they will not have a file hash available. File hashes were being used as "key" in the texture cache (GetTextureIntention are the actual key, but they compare using a hash partially formed by the file hash). Now either the hash or the source URL is used to form the key/hash, depending on whether the file hash is available.

* bumped cache version for landscape (#2515)

## What does this PR change?
see title 

## How to test the changes?

check if [#2190](#2190) is fixed

---------

Signed-off-by: Juan Ignacio Molteni <[email protected]>
Co-authored-by: Gabriel Díaz <[email protected]>
Co-authored-by: Nicolas Lorusso <[email protected]>
Co-authored-by: Geoff Birch <[email protected]>
Co-authored-by: Juan Ignacio Molteni <[email protected]>
Co-authored-by: Alex Villalba <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Pravus <[email protected]>
Co-authored-by: Vitaly Popuzin <[email protected]>
Co-authored-by: Mikhail Agapov <[email protected]>
Co-authored-by: lorenzo-ranciaffi <[email protected]>
Co-authored-by: Alejandro Alvarez Melucci <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants