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

Vertex color only when used #2017

Merged
merged 18 commits into from
Oct 23, 2023
Merged

Vertex color only when used #2017

merged 18 commits into from
Oct 23, 2023

Conversation

julienduroure
Copy link
Collaborator

Export Vertex Color only when used in shader node tree
See #1740

Still WIP, need probably some adjustments

@julienduroure julienduroure added this to the Blender 4.1 milestone Sep 30, 2023
@julienduroure julienduroure marked this pull request as ready for review October 11, 2023 12:34
@julienduroure
Copy link
Collaborator Author

@emackey : Do you get time to have a look on this PR?

@emackey
Copy link
Member

emackey commented Oct 17, 2023

I tried this in 4.0.0 Beta, but I'm struggling to round-trip the VertexColorTest model. It imports OK but the vertex colors appear lost on export.

@julienduroure
Copy link
Collaborator Author

@emackey : I understand why. I didn't manage yet the case where Color Attribute node keeps the layer name empty (will use the active color layer).
For now, you can test by setting the name in the node.
Working on an update for taking active VC into account

@emackey
Copy link
Member

emackey commented Oct 18, 2023

Sorry, looks like a test failure happened on the last commit.

  1. Exporter
    blender_export
    00_looping_animation:

File "/opt/blender/4.1/scripts/addons/io_scene_gltf2/blender/exp/gltf2_blender_gather_primitives_extract.py", line 37, in extract_primitives
primitive_creator.manage_material_info() # UVMap & Vertex Color
File "/opt/blender/4.1/scripts/addons/io_scene_gltf2/blender/exp/gltf2_blender_gather_primitives_extract.py", line 437, in manage_material_info
if material_info['vc_info']['color_type'] is None and material_info['vc_info']['alpha_type'] is None:
KeyError: 'color_type'

@julienduroure
Copy link
Collaborator Author

I was on it, should be fixed now

Copy link
Member

@emackey emackey left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@julienduroure julienduroure merged commit 05dd8a1 into main Oct 23, 2023
1 check passed
@emackey emackey deleted the vertex_color_only_when_used branch October 23, 2023 13:25
@Fenreliania
Copy link

This change has broken some very basic game development workflows without any good recourse. Vertex Colour is frequently used as shader data in a game development context, usually for vertex displacement or texture blending in real-time shaders.

If the engine supports it, one could target the now custom attribute _COLOR_0, but not all engines do, and you now have to enforce this standard for all models, including third party assets. Aside from engines, most other programs also quite rightly look for vertex colour in the standard property.
One could create a custom material that arbitrarily includes the vertex colour every time one creates a model to trick the exporter into actually including the data, but aside from introducing a lot of tedious work and bug hunting, it's just not really sensible that one should be tricking the engine into exporting core data.

Ultimately, the actual problem raised in #1740 was that it was easy to inadvertently pass through vertex colours, because Blender doesn't clearly display them unless you explicitly look for them. This is a UX issue, not something that should have been solved by introducing nonstandard, invisible, unalterable, default behaviour in the exporter. Please revert or otherwise fix this behaviour.

@julienduroure
Copy link
Collaborator Author

@lentsius-bark

before, i could have a material that used vertex colors that multiply a texture which multiplies a colour, however, I'm not sure how to get this to work, this material:

This is already reported in #2110 , should be fixed for 4.2

@TaraSophieDev
Copy link

This also broke my gamedev workflow. Wouldn't it be better if on the export gltf settings vertex colors are toggleable?

@julienduroure
Copy link
Collaborator Author

@RPicster @Arnklit @TaraSophieDev

This should be back under option for Blender 4.2, see #2195 for more information

@julienduroure
Copy link
Collaborator Author

@Arnklit

Having the exporter do this hidden behaviour

For me, this is the opposite. The behavior before 4.1 export Vertex Color that are not shown in Blender viewport. This was an hidden exported feature.
Now data exported are inline with what Blender viewport shows.

Anyway, some option toggles (once again, more and more options...) will let you choose how to export VC in Blender 4.2

@LauraWebdev
Copy link

Oh THATs why stuff broke on my end! Yeah, hopeful the setting is a viable solution, this really introduced some unnecessary debugging time on my end.

@Arnklit
Copy link

Arnklit commented Apr 13, 2024

For me, this is the opposite. The behavior before 4.1 export Vertex Color that are not shown in Blender viewport. This was an hidden exported feature.
Now data exported are inline with what Blender viewport shows.

I expect the data on the mesh to be exported, not what Blender displays in the viewport. UV2 is exported even if it isn't used in the material. Should that be stripped out now as well?

EDIT: Sorry didn't mean to come off as to aggressive here. This is just a really bad change in my mind. happy as long as we get options to resolve this in 4.2.

@julienduroure
Copy link
Collaborator Author

I expect the data on the mesh to be exported, not what Blender displays in the viewport. UV2 is exported even if it isn't used in the material. Should that be stripped out now as well?

Don't think this is the same case

  • TEXCOORD_X is exported, but without any consequences if no material texture uses it.
  • COLOR_0 must act as a multiplier of base color

@julienduroure
Copy link
Collaborator Author

Hello,

To all people reading this because they have issue on Blender 4.1 with Vertex Color export:
We know that the changes may break some workflow that use Vertex Color.
You can read all this conversation to understand why this change happen.

For a reminder: glTF use COLOR_0 as a multiplier for base color. If you are not using VC in your node tree, Blender Viewport does not show it the same way glTF exported it in any Blender version prior to 4.1

I will stop answering individually on each request/remark/complain on this subject (once again, we hear you), in order to spend time on a solution itself.

Thanks again for using this I/O :)

@digimbyte
Copy link

digimbyte commented Apr 13, 2024

Just needs to be an export option to enable or not. Blender is not an authority of a pipeline based on internal shaders. Technical work flows require control.

Just add a checkbox and force vertex if enabled with its type as needed. This way everyone wins.

@padreputativo
Copy link

Do you think it's okay to go around with that photo representing Blender while you mess with multiple industries? Do you think people's lives are a joke or what the hell is wrong with you?

@Sukritact
Copy link

Surprised you’d push this without a solution in place: like I understand why you’re making the change to not export COLOR_0 by default if it’s not being used as base color, but it’s honest sorta strange to push this when a lot of people NEED vertex colors for other reasons

@Rytelier
Copy link

image
This is how I use vertex color in a material, the change requires specific naming to it AND the attribute node in shader editor to be connected to multiply mix node, which is completely incompatible with how I use the material. (this is a blend 4 textures shader where each color represents a texture)
Not to mention I'd have to rename every attribute in every object to match the expected naming convention.
To export my meshes correctly I had to open the scene in version 4.0 and export from there.
I expect this to be fixed by allowing to export vertex color without need for specific nodes and without need for specific naming.

@emanvidmaker
Copy link

if this gets change gets undone//patched out;

i think its worth making a public hotfix/patched build of this while we wait for 4.2
beacuse more and more people are going to notice and get angry and post complains here.

without really understanding how open source works and it might be a headache for contributors

@limarest
Copy link

This change should be reverted as it conflicts with the established workflows of many people.

Under this logic, not using every UV channel in material should also delete unused UV channels. I see this as problematic.

@perkele1989
Copy link

this is the dumbest commit in the history of the game development industry

  1. vertex color attributes arent only used for colors, most of the time theyre used for other stuff
  2. even then, exported models actually rarely have materials setup in blender

Khronos along with committees like the C++ committee have slowly become batshit insane. Please, find common ground with the industries you're working with again, because you've lost it along the way.

I'm not sorry I went with fbx over gltf, despite being encouraged otherwise.

@Takanu
Copy link

Takanu commented Apr 14, 2024

I just wanted to thank you for all your work, while this is an important feature and the concern from some people is understandable, open source development is often thankless work and you often only hear from people when something goes wrong rather than when things go right.

Your addon has provided immense value to me in my daily life and I sincerely appreciate it.

@mikeskydev
Copy link
Contributor

mikeskydev commented Apr 14, 2024

I think the drive for perfect PBR sometimes forgets the hacky and beautiful realities of day to day gamedev, I have some issues with how the glTF spec wants to ignore certain blendOps because "that wouldn't happen in the real world." I have hope the working group will change that with the proposed material graph and texture graph extensions, however.

I do think this needs a hotfix rather than waiting for 4.2.

@julienduroure
Copy link
Collaborator Author

julienduroure commented Apr 15, 2024

Hello,

Let's try to summarize the situation here:

  • In the common usage / historically / simplicity of usage, Vertex Colors are used not only for Vertex Color, but also for transmission of other custom data.
  • The glTF Specification propose a way to transmit custom attributes, but seem for a number of reasons, this workflow is not commonly used:
    • Not very well know
    • Not perfect from Blender side yet (need to use name starting with underscore to avoid exporting internally used custom attributes)
    • Lack of implementation / easy way to retrieve data for some engine/viewer
      This make the end to end workflow of custom attributes not very used (from what I know)

All of these make that VC are commonly used for custom attributes transmission too, even if this goes against the glTF Specification, which says that VC must act as a multiplier of Base Color.

This PR proposed a change that makes VC more accurate when VC are used the way glTF specification requests it, but breaks all other usage of the VC.

Setting some custom attributes in VC COLOR_0 will generate some valid glTF files, but a fully compliant glTF 2.0 viewer must use these data as a base color multiplier, probably resulting in weird shading.

We will come back to you later with more information about how we should solve this together.

edit: spelling

@eugeneko
Copy link

@julienduroure can old behavior be opt-in in export options?

@perkele1989
Copy link

perkele1989 commented Apr 15, 2024

@julienduroure if I'm going to respond in this thread in a more civilized and constructive matter, to bring my experience into the issue:

I believe the glTF specification itself is really what becomes the core issue here. glTF does not simply specify data storage, but it also seems to specify how this data is supposed to be used, as well as how to render meshes that it stores. This makes it fully incompatible with the majority of game engines and games, except for the most simple ones that strictly will follow glTF in all of it's rendering (< 0.01% if I were to take a stab).

So, from this light, criticizing Khronos or glTF developers for this commit becomes kind of stupid and pointless. The real issue is that game developers try to use, or simply condone using, glTF for game development. It doesn't seem to be designed for it.

Or, if glTF is designed to be used for game engines, then instead the problem becomes that the specification itself is fundamentally flawed. Trying to write a specification that truly unifies mesh data across platforms would always be in vain. colors and texture coordinates are the two vertex attributes that one uses as "generic" attributes in game development. If you want to support generic attributes, then game engines (unreal) would also need to support generic attributes. If you try to do it yourself with an explicit gltf attribute in an engine like Unreal, you basically have to write your own mesh importer, and make other heavy modifications, which is unfeasible in such an engine, that already has so much complexity.

So, to sum up; Don't blame this commit. Either blame the specification, or blame developers who want to use this format for game development.

@perkele1989
Copy link

Thus, I apologize for my misdirected anger. Thank you for supporting the open source ecosystem.

@LauraWebdev
Copy link

@perkele1989 It may be true that usage is not "to-spec", but I don't think breaking hundreds of peoples workflows just "to be right" is the right approach. It's disturbing users, especially when "settings to restore the current wildly used workflow" were not even in this PR.

@padreputativo
Copy link

Having coherence and a standardized way of doing things in the world of 3D has been lacking for decades. The limitations of other formats (such as fbx) are what have led to these kinds of "customs". GLTF is a 3D asset exchange format, it is neither intended for video games nor not intended for them. If complete compatibility with everything done in the 3D editor is sought, it's better to use USD. If GLTF allows for an alternative way of performing basic tasks such as having associated extra information, it's the logical path to take, which is what is intended, standardizing 3D exchange. Not allowing everyone to do as they please like an elephant in a china shop while wearing a funny hat as it seems every time I enter this forum... Anything that leads to standardization will be beneficial for everyone. Slow and perhaps traumatic but ultimately, beneficial.

@bitegw
Copy link

bitegw commented Apr 15, 2024

For me, this is the opposite. The behavior before 4.1 export Vertex Color that are not shown in Blender viewport. This was an hidden exported feature.
Now data exported are inline with what Blender viewport shows.

Going by this definition only - if I export the model while in a non-material view the exported model should not have materials. If my armature modifier is temporarily hidden then it shouldn't export the bone weights, and so on.

If we apply this further to appearance, then if I prototype a shader in blender then I guess gltf should store some kind of universal shadercode so that when I import it to a game engine it will still try to match the blender viewport...

Since not exporting 'unused data' is what we're going for, If a mesh/object is entirely inside another let's not include it in the exported file since the user can't see it anyway. Surely the user just didn't know what they were doing. Why would the gltf file format decide this?

But this argument is wrong in the first place, It's already possible to view vertex colors in the viewport without any material set on the model, by setting the 'Viewport shading > Color' option to 'Attribute'. This would lead to a user viewing their model with vertex colors clearly visible in their blender viewport, then exporting it only to find them completely gone. So even users who were using this property as you intended, but didn't need to set up materials in their workflow and used vertex color to paint the model, now have to go through the extra step of adding a material to every object.

Regardless, it's crazy that this PR didn't include a fallback export setting. I've seen many more examples of using vertex colors for other uses than as actual colors by a large margin. You could put a blaring red outline around the option and describe that this is not how it's meant to be used, and I'm pretty sure everyone who does - still would.

@Takanu
Copy link

Takanu commented Apr 17, 2024

@julienduroure Over a long enough timeframe custom attributes should do this job better, but I thought it'd be worth mentioning that one of the reasons artists use Vertex Colors to smuggle data is that it's a very standard feature in game engines, and if you don't have a programmer or engineer that's building pipeline tools for you then you have to use what's available.

For me working in Unity for example I can access Vertex Colors and UVs in shaders and do with them what I like (and I use both frequently to pass data to shaders), but I can't access custom attributes, not without messing with the community importer I use to try and plug the custom data into the imported model's UV or Vertex Color data.

It will take a long time for custom attribute support to make its way across the entire pipeline, but in the meantime artists need to use what they have available, and Vertex Colors are a very easy and visual way of getting that done.

@Invertex
Copy link

For those using Godot, I've created a workaround for now, since this completely breaks an important part of our rendering...

https://github.com/Invertex/GODOT_Vertex-Color-Import-Swapper

Should show up on the AssetLib once approved though instead of getting it directly from there, so keep an eye out for it!

@bertodelrio256
Copy link

bertodelrio256 commented May 2, 2024

this also broke my lightmapping workflow for my game! I have preview materials setup in blender that use the vertex color in a way that allows me to preview what it will look like in game. much like @Rytelier , I have terrain materials that use the vertex color channels for texture blending and the alpha channel for ambient occlusion which i bake using a script. its critical to be able to preview the painted masks as you change them.

this is what my material node setup looks like and the vertex color is split up and routed all over the place:
image

@julienduroure
Copy link
Collaborator Author

Dear all,
Here is a proposition for 4.2, where we give users an option to export active VC, even if not used in the node tree.
#2233

@Underwater008
Copy link

Underwater008 commented Jun 1, 2024

Dear all, Here is a proposition for 4.2, where we give users an option to export active VC, even if not used in the node tree. #2233

Is there a correct way to use vertex color in the node tree so it will be exported without affecting or changing anything?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.