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

[3D] Unify some some shader code #58114

Merged
merged 5 commits into from
Jul 18, 2024
Merged

Conversation

ptitjano
Copy link
Contributor

Description

This PR is factored out from #57899
It contains some unification for the shader code:

  • Use the default vertex code for QgsPhongTexturedMaterialSettings
  • Use the default vertex code for QgsPhongMaterialSettings
  • Merge the constant and data defined cases of QgsPhongMaterialSettings with one shader file by using a #ifdef logic
  • Merge the constant and data defined cases of QgsGoochMaterialSettings with one shader file by using a #ifdef logic

@github-actions github-actions bot added this to the 3.40.0 milestone Jul 15, 2024
@ptitjano ptitjano self-assigned this Jul 15, 2024
@ptitjano ptitjano added the 3D Relates to QGIS' 3D engine or rendering label Jul 15, 2024
Copy link

github-actions bot commented Jul 15, 2024

🪟 Windows builds ready!

Windows builds of this PR are available for testing here. Debug symbols for this build are available here.

(Built from commit 000e4e9)

Copy link
Contributor

@benoitdm-oslandia benoitdm-oslandia left a comment

Choose a reason for hiding this comment

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

LGTM!

There is no need to define a special vertex shader code for this
material, it can use the default case. This way, it reduces the number
of vertex shader files.
There is no need to define a special vertex shader code for this
material, it can use the default case. This way, it reduces the number
of vertex shader files.
This will be used by `QgsPhongMaterialSettings` in the next commit.
This commit makes two changes to `QgsPhongMaterialSettings`:
1. It merges `dataDefinedMaterial()` and `constantColorMaterial` into
one new function: `buildMaterial`.

2. The fragment shader files `phongDataDefined.frag` and
`phongConstant.frag` are merged into a new file `phong.frag`. This is
achieved by a `#define` logic. If `DATA_DEFINED` is defined, this is
the dataDefined case. Otherwise, this is the constant case.
@ptitjano ptitjano closed this Jul 18, 2024
@ptitjano ptitjano reopened this Jul 18, 2024
This is similar to the previous QgsPhongMaterialSettings change:

This commit makes two changes to `QgsGoochMaterialSettings`:
1. It merges `dataDefinedMaterial()` and `constantColorMaterial` into
one new function: `buildMaterial`.

2. There is one fragment shader file called `gooch.frag`. This is
achieved by a `#define` logic. If `DATA_DEFINED` is defined, this is
the dataDefined case. Otherwise, this is the constant case.
@nyalldawson nyalldawson merged commit 58928f6 into qgis:master Jul 18, 2024
28 checks passed
@wonder-sk
Copy link
Member

good stuff! 👏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3D Relates to QGIS' 3D engine or rendering
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants