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

GeometryInstance3D transparency does not work in mobile or compatibility renderers #76774

Open
Tracked by #55871 ...
fracteed opened this issue May 6, 2023 · 5 comments
Open
Tracked by #55871 ...

Comments

@fracteed
Copy link

fracteed commented May 6, 2023

Godot version

4.0.2 stable

System information

win10 960m + win11 3050

Issue description

The transparency parameter on a geometry instance 3d object only works in the forward+ renderer. It does not work with the vulkan mobile or opengl compatibility renderers.

@clayjohn has confirmed that it has not been added to these renderers.

Steps to reproduce

  • add a cube mesh
  • move the transparency slider

Minimal reproduction project

Not really necessary.

@Calinou
Copy link
Member

Calinou commented May 6, 2023

This also applies to visibility range distance fade using the Self or Dependencies fade mode. Both act the same as Disabled when using the Forward Mobile or Compatibility rendering methods (#69877).

Porting GeometryInstance3D transparency and distance fade to work with the Forward Mobile rendering method should be straightforward, but it'll be more involved for Compatibility.

This line in scene_forward_clustered.glsl should give you a pointer for doing so:

float alpha = float(instances.data[instance_index].flags >> INSTANCE_FLAGS_FADE_SHIFT) / float(255.0);

You will have to modify scene_forward_mobile.glsl.

@eswartz
Copy link
Contributor

eswartz commented Jan 15, 2024

I spent a few hours before discovering this issue. I might recommend documenting this limitation . The GeometryInstance3D.transparency (etc) docs don't mention that these are only available in Forward+.

@jbromberg

This comment was marked as off-topic.

@eswartz
Copy link
Contributor

eswartz commented May 3, 2024

BTW, should this ticket be replicated into #55871? Currently it's only on the OpenGL/compatibility tracker.

@Calinou
Copy link
Member

Calinou commented May 6, 2024

BTW, should this ticket be replicated into #55871? Currently it's only on the OpenGL/compatibility tracker.

Done.

eswartz added a commit to eswartz/godot that referenced this issue May 7, 2024
This is mostly a copy-paste of similar changes in the Forward+ renderer,
as hinted in godotengine#76774 (comment) and godotengine#87231 (review),
along with a tweak to the shader and pipeline model.

The shader pipeline tweak involves distinguishing geometry instance-enabled
transparency from material-enabled transparency. A material cannot support
transparency on a per-instance level; a separate pipeline must be used when rendering.

Originally in c571e4a this was a non-trivial refactoring in Forward+
(which in the past few years has evolved a few more times to account for the multitude of combinations
allowed there).

I opted to follow a similar sentiment to `SceneShaderForwardClustered::PipelineColorPassFlags`
in a simpler `SceneShaderForwardMobile::PipelinePasses` enum, which only accounts
for don't-care or opaque pipelines vs. color transparency-enabled ones.

This multiplies the number of `PipelineCacheRD`s by two, but otherwise accomplishes
the goals without adding unnecessary changes or shooting myself or others in the foot (I hope).
eswartz added a commit to eswartz/godot that referenced this issue May 7, 2024
This is mostly a copy-paste of similar changes in the Forward+ renderer,
as hinted in godotengine#76774 (comment) and godotengine#87231 (review),
along with a tweak to the shader and pipeline model.

The shader pipeline tweak involves distinguishing geometry instance-enabled
transparency from material-enabled transparency. A material cannot support
transparency on a per-instance level; a separate pipeline must be used when rendering.

Originally in c571e4a this was a non-trivial refactoring in Forward+
(which in the past few years has evolved a few more times to account for the multitude of combinations
allowed there).

I opted to follow a similar sentiment to `SceneShaderForwardClustered::PipelineColorPassFlags`
in a simpler `SceneShaderForwardMobile::PipelinePasses` enum, which only accounts
for don't-care or opaque pipelines vs. color transparency-enabled ones.

This multiplies the number of `PipelineCacheRD`s by two, but otherwise accomplishes
the goals without adding unnecessary changes or shooting myself or others in the foot (I hope).

BTW I also reorganized and removed some redundant checks in
`RenderForwardClustered::_geometry_instance_add_surface_with_material`
(since I was comparing this side-by-side with the `RenderForwardMobile` version, which seems
better-organized). There is no functional change since `has_alpha` is the only variable used
and includes all the checks anyway.
eswartz added a commit to eswartz/godot that referenced this issue May 7, 2024
This is mostly a copy-paste of similar changes in the Forward+ renderer,
as hinted in godotengine#76774 (comment) and godotengine#87231 (review),
along with a tweak to the shader and pipeline model.

The shader pipeline tweak involves distinguishing geometry instance-enabled
transparency from material-enabled transparency. A material cannot support
transparency on a per-instance level; a separate pipeline must be used when rendering.

Originally in c571e4a this was a non-trivial refactoring in Forward+
(which in the past few years has evolved a few more times to account for the multitude of combinations
allowed there).

I opted to follow a similar sentiment to `SceneShaderForwardClustered::PipelineColorPassFlags`
in a simpler `SceneShaderForwardMobile::PipelinePasses` enum, which only accounts
for don't-care or opaque pipelines vs. color transparency-enabled ones.

This multiplies the number of `PipelineCacheRD`s by two, but otherwise accomplishes
the goals without adding unnecessary changes or shooting myself or others in the foot (I hope).

BTW I also reorganized and removed some redundant checks in
`RenderForwardClustered::_geometry_instance_add_surface_with_material`
(since I was comparing this side-by-side with the `RenderForwardMobile` version, which seems
better-organized). There is no functional change since `has_alpha` is the only variable used
and includes all the checks anyway.
eswartz added a commit to eswartz/godot that referenced this issue May 14, 2024
This is mostly a copy-paste of similar changes in the Forward+ renderer,
as hinted in godotengine#76774 (comment) and godotengine#87231 (review),
along with a tweak to the shader and pipeline model.

The shader pipeline tweak involves distinguishing geometry instance-enabled
transparency from material-enabled transparency. A material cannot support
transparency on a per-instance level; a separate pipeline must be used when rendering.

Originally in c571e4a this was a non-trivial refactoring in Forward+
(which in the past few years has evolved a few more times to account for the multitude of combinations
allowed there).

I opted to follow a similar sentiment to `SceneShaderForwardClustered::PipelineColorPassFlags`
in a simpler `SceneShaderForwardMobile::PipelinePasses` enum, which only accounts
for don't-care or opaque pipelines vs. color transparency-enabled ones.

This multiplies the number of `PipelineCacheRD`s by two, but otherwise accomplishes
the goals without adding unnecessary changes or shooting myself or others in the foot (I hope).

BTW I also reorganized and removed some redundant checks in
`RenderForwardClustered::_geometry_instance_add_surface_with_material`
(since I was comparing this side-by-side with the `RenderForwardMobile` version, which seems
better-organized). There is no functional change since `has_alpha` is the only variable used
and includes all the checks anyway.
eswartz added a commit to eswartz/godot that referenced this issue May 28, 2024
This is mostly a copy-paste of similar changes in the Forward+ renderer,
as hinted in godotengine#76774 (comment) and godotengine#87231 (review),
along with a tweak to the shader and pipeline model.

The shader pipeline tweak involves distinguishing geometry instance-enabled
transparency from material-enabled transparency. A material cannot support
transparency on a per-instance level; a separate pipeline must be used when rendering.

Originally in c571e4a this was a non-trivial refactoring in Forward+
(which in the past few years has evolved a few more times to account for the multitude of combinations
allowed there).

I opted to follow a similar sentiment to `SceneShaderForwardClustered::PipelineColorPassFlags`
in a simpler `SceneShaderForwardMobile::PipelinePasses` enum, which only accounts
for don't-care or opaque pipelines vs. color transparency-enabled ones.

This multiplies the number of `PipelineCacheRD`s by two, but otherwise accomplishes
the goals without adding unnecessary changes or shooting myself or others in the foot (I hope).

BTW I also reorganized and removed some redundant checks in
`RenderForwardClustered::_geometry_instance_add_surface_with_material`
(since I was comparing this side-by-side with the `RenderForwardMobile` version, which seems
better-organized). There is no functional change since `has_alpha` is the only variable used
and includes all the checks anyway.
eswartz added a commit to eswartz/godot that referenced this issue Jun 5, 2024
This is mostly a copy-paste of similar changes in the Forward+ renderer,
as hinted in godotengine#76774 (comment) and godotengine#87231 (review),
along with a tweak to the shader and pipeline model.

The shader pipeline tweak involves distinguishing geometry instance-enabled
transparency from material-enabled transparency. A material cannot support
transparency on a per-instance level; a separate pipeline must be used when rendering.

Originally in c571e4a this was a non-trivial refactoring in Forward+
(which in the past few years has evolved a few more times to account for the multitude of combinations
allowed there).

I opted to follow a similar sentiment to `SceneShaderForwardClustered::PipelineColorPassFlags`
in a simpler `SceneShaderForwardMobile::PipelinePasses` enum, which only accounts
for don't-care or opaque pipelines vs. color transparency-enabled ones.

This multiplies the number of `PipelineCacheRD`s by two, but otherwise accomplishes
the goals without adding unnecessary changes or shooting myself or others in the foot (I hope).

BTW I also reorganized and removed some redundant checks in
`RenderForwardClustered::_geometry_instance_add_surface_with_material`
(since I was comparing this side-by-side with the `RenderForwardMobile` version, which seems
better-organized). There is no functional change since `has_alpha` is the only variable used
and includes all the checks anyway.
eswartz added a commit to eswartz/godot that referenced this issue Jun 5, 2024
This is mostly a copy-paste of similar changes in the Forward+ renderer,
as hinted in godotengine#76774 (comment) and godotengine#87231 (review),
along with a tweak to the shader and pipeline model.

The shader pipeline tweak involves distinguishing geometry instance-enabled
transparency from material-enabled transparency. A material cannot support
transparency on a per-instance level; a separate pipeline must be used when rendering.

Originally in c571e4a this was a non-trivial refactoring in Forward+
(which in the past few years has evolved a few more times to account for the multitude of combinations
allowed there).

I opted to follow a similar sentiment to `SceneShaderForwardClustered::PipelineColorPassFlags`
in a simpler `SceneShaderForwardMobile::PipelinePasses` enum, which only accounts
for don't-care or opaque pipelines vs. color transparency-enabled ones.

This multiplies the number of `PipelineCacheRD`s by two, but otherwise accomplishes
the goals without adding unnecessary changes or shooting myself or others in the foot (I hope).

BTW I also reorganized and removed some redundant checks in
`RenderForwardClustered::_geometry_instance_add_surface_with_material`
(since I was comparing this side-by-side with the `RenderForwardMobile` version, which seems
better-organized). There is no functional change since `has_alpha` is the only variable used
and includes all the checks anyway.
eswartz added a commit to eswartz/godot that referenced this issue Jun 6, 2024
This is mostly a copy-paste of similar changes in the Forward+ renderer,
as hinted in godotengine#76774 (comment) and godotengine#87231 (review),
along with a tweak to the shader and pipeline model.

The shader pipeline tweak involves distinguishing geometry instance-enabled
transparency from material-enabled transparency. A material cannot support
transparency on a per-instance level; a separate pipeline must be used when rendering.

Originally in c571e4a this was a non-trivial refactoring in Forward+
(which in the past few years has evolved a few more times to account for the multitude of combinations
allowed there).

I opted to follow a similar sentiment to `SceneShaderForwardClustered::PipelineColorPassFlags`
in a simpler `SceneShaderForwardMobile::PipelinePasses` enum, which only accounts
for don't-care or opaque pipelines vs. color transparency-enabled ones.

This multiplies the number of `PipelineCacheRD`s by two, but otherwise accomplishes
the goals without adding unnecessary changes or shooting myself or others in the foot (I hope).

BTW I also reorganized and removed some redundant checks in
`RenderForwardClustered::_geometry_instance_add_surface_with_material`
(since I was comparing this side-by-side with the `RenderForwardMobile` version, which seems
better-organized). There is no functional change since `has_alpha` is the only variable used
and includes all the checks anyway.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants