Skip to content

Add shadows_disabled macro in Compatibility renderer#84416

Merged
akien-mga merged 1 commit into
godotengine:masterfrom
jsjtxietian:shadow_disabled
Dec 4, 2023
Merged

Add shadows_disabled macro in Compatibility renderer#84416
akien-mga merged 1 commit into
godotengine:masterfrom
jsjtxietian:shadow_disabled

Conversation

@jsjtxietian

Copy link
Copy Markdown
Contributor

Fixes #84386

@jsjtxietian jsjtxietian requested a review from a team as a code owner November 3, 2023 15:49
@YuriSizov YuriSizov added this to the 4.2 milestone Nov 3, 2023
@clayjohn clayjohn modified the milestones: 4.2, 4.3 Nov 7, 2023
@clayjohn clayjohn added the cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release label Nov 7, 2023

@clayjohn clayjohn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Seems fine to me.

Let's merge this for 4.3 as we are in the final stages of 4.2. We can cherrypick this for 4.2.1

@AThousandShips AThousandShips left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Some accidental changes that should be restored

Comment thread drivers/gles3/shaders/scene.glsl Outdated
Comment thread drivers/gles3/shaders/scene.glsl Outdated

@Calinou Calinou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Tested locally. This PR breaks rendering in an empty project with a few meshes, even if they have no material assigned: test_pr_84416.zip

ERROR: SceneShaderGLES3: Fragment shader compilation failed:
0(1540) : error C7102: unmatched #endif

   at: _display_error_with_code (drivers/gles3/shader_gles3.cpp:252)

@jsjtxietian

jsjtxietian commented Nov 7, 2023

Copy link
Copy Markdown
Contributor Author

Hi @Calinou Did you use the last commit, I do miss a macro there when try to fix the clang format issue.

This one is broken and the latest one is fine:

image

@Calinou Calinou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Tested latest revision, it works as expected. Enabling Disable Receive Shadows on the material correctly disables shadow receiving for that material for all light types.

@jsjtxietian

Copy link
Copy Markdown
Contributor Author

Thanks! I will try not to update my llvm too aggressively next time :(

@akien-mga akien-mga changed the title Add shadows_disabled macro in Compatibility renderer Add shadows_disabled macro in Compatibility renderer Dec 4, 2023
@akien-mga akien-mga merged commit 6f16e3f into godotengine:master Dec 4, 2023
@akien-mga

Copy link
Copy Markdown
Member

Thanks!

@YuriSizov

Copy link
Copy Markdown
Contributor

Cherry-picked for 4.2.1.

@YuriSizov YuriSizov removed the cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release label Dec 5, 2023
@jsjtxietian jsjtxietian deleted the shadow_disabled branch December 9, 2023 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Shadows cannot be disabled in the material in the Compatibility renderer

6 participants