Skip to content

Remap alpha channel of baseColorMap #6479

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

Merged
merged 10 commits into from
Jan 13, 2022
Merged

Conversation

skhiat
Copy link
Contributor

@skhiat skhiat commented Dec 5, 2021

Ref:
#6201
(already reviewed)

Remap Transparent Channel of the BaseColor/Color texture for {Lit, Unlit, LayeredLit, LitTesselation, LayeredLitTesselation}
Lit:
image

Unlit:
image

LayeredLit:
image

LitTesselation:
image

LayeredLitTesselation:
image

@github-actions
Copy link

github-actions bot commented Dec 5, 2021

Hi! This comment will help you figure out which jobs to run before merging your PR. The suggestions are dynamic based on what files you have changed.
Link to Yamato: https://unity-ci.cds.internal.unity3d.com/project/902/
Search for your PR branch using the search bar at the top, then add the following segment(s) to the end of the URL (you may need multiple tabs depending on how many packages you change)

HDRP
/jobDefinition/.yamato%2Fall-hdrp.yml%23PR_HDRP_trunk
With changes to HDRP packages, you should also run
/jobDefinition/.yamato%2Fall-lightmapping.yml%23PR_Lightmapping_trunk

Depending on the scope of your PR, you may need to run more jobs than what has been suggested. Please speak to your lead or a Graphics SDET (#devs-graphics-automation) if you are unsure.

@github-actions github-actions bot added the HDRP label Dec 5, 2021
@skhiat skhiat requested a review from kecho December 6, 2021 00:04
Copy link
Contributor

@Vic-Cooper Vic-Cooper left a comment

Choose a reason for hiding this comment

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

These docs changes are consistent with the existing docs, so no changes needed!

@skhiat skhiat marked this pull request as ready for review December 6, 2021 10:35
Copy link
Contributor

@kecho kecho left a comment

Choose a reason for hiding this comment

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

Lets remerge, rerun yamato. Looks good from my side

# Conflicts:
#	com.unity.render-pipelines.high-definition/Documentation~/Lit-Tessellation-Shader.md
@sebastienlagarde sebastienlagarde requested a review from a team December 6, 2021 12:55
Copy link
Contributor

@sebastienlagarde sebastienlagarde left a comment

Choose a reason for hiding this comment

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

This don't work correctly with alpha test. See comment in unlit.hlsl

Also you haven't udpate the LitData.hlsl
here: float alphaValue = SAMPLE_UVMAPPING_TEXTURE2D(_BaseColorMap, sampler_BaseColorMap, layerTexCoord.base).a * _BaseColor.a;

@sebastienlagarde
Copy link
Contributor

For QA: think to check the slider with alpha test

@RemyUnity
Copy link
Contributor

RemyUnity commented Dec 8, 2021

Should we consider adding a graphic test for this ?
(or adding coverage of this in existing test(s))

@RemyUnity RemyUnity requested review from RemyUnity and removed request for a team December 8, 2021 14:50
Copy link
Contributor

@RemyUnity RemyUnity left a comment

Choose a reason for hiding this comment

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

The changes are ok for me, I roughly tested it on my side but @remi-chapelain already covered all materials in the original PR.
Now it just needs coverage graphic tests (or maybe can be merged already if we don't forget to do it later on @sebastienlagarde )

…HDRP/remap_alpha_channel

# Conflicts:
#	com.unity.render-pipelines.high-definition/Runtime/Material/Unlit/UnlitProperties.hlsl
@skhiat skhiat requested a review from RemyUnity January 11, 2022 13:19
@sebastienlagarde sebastienlagarde merged commit c9dfc73 into master Jan 13, 2022
@sebastienlagarde sebastienlagarde deleted the HDRP/remap_alpha_channel branch January 13, 2022 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants