EEVEE-Next: Add operator to convert a world volume to mesh #119734
No reviewers
Labels
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset System
Interest
Audio
Interest
Automated Testing
Interest
Blender Asset Bundle
Interest
BlendFile
Interest
Code Documentation
Interest
Collada
Interest
Compatibility
Interest
Compositing
Interest
Core
Interest
Cycles
Interest
Dependency Graph
Interest
Development Management
Interest
EEVEE
Interest
Freestyle
Interest
Geometry Nodes
Interest
Grease Pencil
Interest
ID Management
Interest
Images & Movies
Interest
Import Export
Interest
Line Art
Interest
Masking
Interest
Metal
Interest
Modeling
Interest
Modifiers
Interest
Motion Tracking
Interest
Nodes & Physics
Interest
OpenGL
Interest
Overlay
Interest
Overrides
Interest
Performance
Interest
Physics
Interest
Pipeline, Assets & IO
Interest
Platforms, Builds & Tests
Interest
Python API
Interest
Render & Cycles
Interest
Render Pipeline
Interest
Sculpt, Paint & Texture
Interest
Text Editor
Interest
Translations
Interest
Triaging
Interest
Undo
Interest
USD
Interest
User Interface
Interest
UV Editing
Interest
VFX & Video
Interest
Video Sequencer
Interest
Viewport & EEVEE
Interest
Virtual Reality
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Interest: X11
Legacy
Asset Browser Project
Legacy
Blender 2.8 Project
Legacy
Milestone 1: Basic, Local Asset Browser
Legacy
OpenGL Error
Meta
Good First Issue
Meta
Papercut
Meta
Retrospective
Meta
Security
Module
Animation & Rigging
Module
Core
Module
Development Management
Module
Grease Pencil
Module
Modeling
Module
Nodes & Physics
Module
Pipeline, Assets & IO
Module
Platforms, Builds & Tests
Module
Python API
Module
Render & Cycles
Module
Sculpt, Paint & Texture
Module
Triaging
Module
User Interface
Module
VFX & Video
Module
Viewport & EEVEE
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Severity
High
Severity
Low
Severity
Normal
Severity
Unbreak Now!
Status
Archived
Status
Confirmed
Status
Duplicate
Status
Needs Info from Developers
Status
Needs Information from User
Status
Needs Triage
Status
Resolved
Type
Bug
Type
Design
Type
Known Issue
Type
Patch
Type
Report
Type
To Do
No Milestone
No project
No Assignees
8 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: blender/blender#119734
Loading…
Reference in New Issue
Block a user
No description provided.
Delete Branch "Jeroen-Bakker/blender:eevee-next/convert-world-volume"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
EEVEE-Next world volume are infinite like Cycles. EEVEE-Classic world volumes
end at the clip_end of the camera/viewport. This can lead to confusion as
it would render different then expected.
This PR adds an operator to convert a world volume into a mesh volume. The
operator can be found in the shader editor (world mode) and in the properties
panel/World/Volume.
Why an operator?
As this alters the content of the scene we want the artist to be in control of
the conversion. Doing it automatic lead to a lot of complexity and cases that
might not be expected by the user.
I already send this one in for review to discuss the best location where to add the operator in the menus.
This thing permit to create the beginning of probe volume based on the world (Sparse grid LIKE adaptive probe volume) (for using probe with surfels instead raytracing or, with raytracing) ?
So we had a discussion with Pablo about this and we both think it is better to go with the versioning approach. The reason is that users should not have to make any input to see their file work in newer version.
So we listed made a list of things to do in order to make it the cleanest as possible.
@fclem @pablovazquez I have doubts about this solution. As this is different than previously asked to do and the design doesn't cover all scenarios.
User perspective:
Developer perspective:
So basically the proposal isn't complete as it only covers the basics. I expect the total lead time can go easily into 3-4 weeks, development including support. We might even come into a situation where we don't want this as the expectation for users are different, but they need to do additional work.
All in all I doubt this proposal will fit the schedule or works for the users as intended.
I share this concern. But even if the list of things to do is long, it isn't very hard to do for the most part. The only tricky part is dealing with nodetrees.
We should only convert worlds local to this file, which is safer for data management. This mean users will have to fix file in inclusion order. Which I think is sound.
If a user opens an asset file that has a world volume, saves it, and then load a set file that link the world data block, in that instance, the first generated volume world collection will not be automatically linked to the final set and it would miss the world volume. Which I think is better than rendering black world.
If a user opens a file with a linked scene, then no conversion happens and the world will render black.
In any case, I would expect this is something only happening in larger production pipelines where a pipeline TD will have to fix manually. But even then, EEVEE-Next is supposed to be disruptive, so I don't think users expect these complex cases to work without modifications.
There will be a report that a conversion happened. Making the user aware that there is modification.
The volume objects are visible in the outliner, making it possible to either fix or detect the conflict.
Hiding an operator somewhere add a burden on the user to make their file work (by default, not only for failure case) and adds a discoverability issue.
The failure cases are:
The procedure for each would be:
Which I think is not so bad.
There is plenty of area in the versioning code that just fails silently. We should make a report that indicates the world volume versioning failed. It think it is just a matter of communication. But that's the same issue as with the operator, the user have to take action and have to know what. But at least it is not for the common case.
Also we do these kind of destructive conversions quite often (node replacement, convert autosmooth to modifier, etc...). If they are documented and there is a notification, they are usually well received.
We do increase the minimum blendfile version blender can load once in a while.
TL;DR
Doing the versioning only local worlds (not linked) will fix most cases while make it trivial to fix others.
If we only convert local data, than it most likely doesn't fit as part of versioning code. I am not aware of versioning code where we make this differentiation, I would prefer to have some approval from brecht/bastien about that, making sure we don't do something that needs to be rolled back because it has many issues.
I can imagine we add it to the append/open on a higher level, but it is also awkward due to the duration the code would commonly run.
For we also introduce 2 different results (link vs append) which is IMO bad.
The issue is that ID data blocks are user owned. Conversions you refer to happen within a data block so is fine and manageable.
@brecht @mont29 This is a very touchy topic about adding ID's during versioning and selectively do this from file local IDs (not linked). We need your insights.
Ow BTW I do think using a collection and how the ico sphere should be done is a good idea.
I would not accept a solution that handle half of the conversion problem automatically, and leaves the other half for the user to fix manually, in a cryptic and absolutely non-obvious way from Blender usage PoV (even if its clearly documented in the release notes - who reads them? ;) ).
I would also rather avoid the fully manual solution, as it would also most likely be dodgy (at best) in linking/appending cases.
I am not sure I understand why you want to skip linked data in your current proposal? We now (should!) have all the tooling to create 'virtual' linked data during versioning. You can see e.g. how GPv3 code now deals with the goenodetree it needs to create for the legacy GPv2 layer thickness adjustments (see
offset_radius_node_tree_add()
and its usage ingrease_pencil_convert_legacy.cc
). And the new EEVEE conversion code should almost certainly be called at the same places asblender::bke::greasepencil::convert::legacy_main()
.Ok, so if we do have a way to have virtual linked collections (and all the other needed datablocks), then I don't see any issue with linked scenes.
What happens in the case of a linked world in a scene that isn't linked? If we can add a linked (virtual) collection to the scene, then it would also fix the second problem.
The only thing I'm not sure about this is what happens if you have recursive linking. Does the versionning applies after all linking is done or is it done after each linking step? and would it affect the outcome?
Also I hope overrides are not possible on these versionned datablocks, otherwise that could become quite bad.
Also final issue, what about unused worlds (not used by any scenes)? Should we create collections for them too?
Yes once created, these linked IDs behave like any other linked IDs, so you can use them in your local data as well. The only thing is that they do not exist, on next file read, their local usages will be reset to
nullptr
, and they will be re-generated afterwards.This versioning (from
do_version_after_setup()
) applies at the end of the readfile process, once every thing has been read, linked, and the more 'basic' doversion has been done. In other words, on the Main data-base is fully 'ready'.They are. But they would be useless, since these generated linked data are re-created after liboverride is applied in readfile process. It might be worth adding a runtime tag to these virtual linked IDs to prevent that kind of thing actually... We could also prevent showing them in the list of usable IDs altogether (similar to @brecht idea to filter out some IDs from the 'immediately user-visible' ones).
That I don't know... Creating the collection is fine, but then how would the user know that they have to add it to their viewlayer when they decide to use the matching world?
Virtual collections solves most issues. Manual steps are needed when considering conversion order. In case of grease pencil this might not have been an issue as it is all part of the same ID.
In case we link directly to a world. And the file containing the world is converted. Any file linking to that world (converted or not), doesn't know which collection to include. Storing a ref in the world cannot work as the versioning code will not be triggered anymore.
A limitation is that world assets will look different (when dragged/dropped) before and after the conversion.
Seems to be a similar case that Bastien mentions in previous post, except that it can happens for any worlds.
So perhaps the decision is how we inform the user. Of course release-notes. But it is hard to communicate it when the user need to act. Asset libraries can be altered by someone else.
My advice would be to not add any versioning or operator, and accept the compatibility breakage. I don't think all this complexity is worth it.
Not agree with you in future blender vesions this thing can be needed !
I had a discussion with @dfelinto on the subject. He mentioned that we had this issue of adding manual conversion in the past. Apparently this was done by adding the operator in the help menu in the top bar. I don't know if this still fits nowadays UI guidelines but that sounds a good location for it.
I don't think just accepting the breakage without an easy way to fix the file is the best option. Even if we document it, the steps to make a file work are quite involved. So I would still think an operator is kind of a good compromise. We can just link to it in the release notes (instead of listing a whole complex procedure).
I'm not sure there's really a good place to make that kind of operator discoverable. If we can make the operator only appear when there is known to be an old world volume in the file, it can be more prominent. But still I don't know if anyone would look in the Help menu for that.
If we are going to have to guide users to it through release notes and docs, maybe it is best to put it in a logical place, like the shader editor header when a world is selected there, and the volume panel in the world properties editor.
The thing is, this is not trivial. Except if we know from which blendfile version the datablock is.
We can add a flag to the world datablock in versioning for that?
@ -0,0 +14,4 @@
if not world or not world.use_nodes:
return False
camera = context.scene.camera
camera
is not used in the class. I think it is better not to bail out if no camera is present.@ -0,0 +37,4 @@
# Add World Volume Mesh object to scene
mesh = bpy.data.meshes.new(name)
volume = bpy.data.objects.new(name, mesh)
Rename to
object
@ -0,0 +82,4 @@
world_tree.links.remove(link)
collection.objects.link(volume)
volume.select_set(True)
This raise a good point. The design was to make the object not selectable. But since now the operator is to be triggered by users, that makes more sense.
@pablovazquez any opinion?
Makes sense indeed. +1
@ -3494,6 +3494,10 @@ class VIEW3D_MT_object_convert(Menu):
layout.template_node_operator_asset_menu_items(catalog_path="Object/Convert")
world = context.scene.world
From Brecht's comment:
I think we agreed to expose it in the convert operator.
@ -3350,0 +3352,4 @@
if (!MAIN_VERSION_FILE_ATLEAST(bmain, 402, 28)) {
LISTBASE_FOREACH (World *, world, &bmain->worlds) {
if (world->nodetree) {
world->nodetree->ensure_topology_cache();
Seems like this was never used in versioning code. Would like to have approval of @JacquesLucke to know if that's safe.
Good point. I think it would be good to keep the versioning code a bit more self contained instead of calling into code that might change over time. It's probably safe to do this right now, but not sure if it will stay safe.
With more contained in this sense would mean to duplicate
ntreeShaderOutputNode
or make a variant which can be called during conversion.I chose to duplicate the essential part of the function. But assume that the active output is always set. This was a change made 8 years ago and might be safe to be removed during versioning. Porting this part would require the call to the
ensure_topology_cache
or duplicate the very genericntreeSetOutput
@ -225,0 +228,4 @@
RNA_def_property_ui_text(
prop,
"Finite Volume",
"World uses EEVEE Classic volumes. These volumes are finite by view bounds and needs to be "
The world's volume used to be rendered by EEVEE Legacy. Conversion is needed for it to render properly
The property to toggle infinite EEVEE Volume is not exposed to the GUI right now. User inaccessible.
@AndresStephens The property is only used internally to show up the operator button. There is no need for it to be displayed.