Page MenuHome

Freestyle render removes texture image user
Closed, ResolvedPublicBUG

Description

System Information
Operating system: Windows-10-10.0.18362-SP0 64 Bits
Graphics card: GeForce GTX 1660 Ti/PCIe/SSE2 NVIDIA Corporation 4.5.0 NVIDIA 441.41

Blender Version
Broken: version: 2.82 (sub 7), branch: master, commit date: 2020-02-12 16:20, hash: rB77d23b0bd76f
Worked: (optional)

Short description of error
Freestyle linestyle texture goes missing every time I open the file.

Exact steps for others to reproduce the error

  1. PFA zip file. It has 2 files .blend and png for linestyle.
  2. When you open the file you will notice that there's no texture called in image texture node.
  3. Call the texture image in zip file.
  4. Render and it should render freestyle lines.
  5. Save and close the file.
  6. Open the saved file again and you will notice texture goes missing in image texture node.

Event Timeline

Brecht Van Lommel (brecht) changed the task status from Needs Triage to Needs Information from User.Mar 4 2020, 8:27 AM

It seems no file got attached?

Ankit (ankitm) renamed this task from Freestyle texture Bug to Freestyle texture not getting saved despite getting rendered.Wed, Mar 4, 2:40 PM

There you go. I think this should work.

Brecht Van Lommel (brecht) renamed this task from Freestyle texture not getting saved despite getting rendered to Freestyle render removes texture image user.Thu, Mar 5, 11:32 AM
Brecht Van Lommel (brecht) changed the task status from Needs Information from User to Confirmed.
Brecht Van Lommel (brecht) triaged this task as High priority.
Jacques Lucke (JacquesLucke) changed the subtype of this task from "Report" to "Bug".Tue, Mar 17, 3:35 PM

I think I found the culprit.

When rendering starts:

  • In BlenderStrokeRenderer::RenderStrokeRepBasic(…) there is a call to GetStrokeShader(…, do_id_user=false).
  • That function reduces the user count of the "stroke_shader" material to zero.
  • Because of do_id_user=false, the ntreeCopyTree_ex() does *not* increase the user count for the copied node tree.

When rendering stops:

  • All copied materials are looped over and freed.
  • Freeing the material recursively frees everything it uses.
  • This leaves the texture node's image with a user count that's one too little.

Now the image datablock has no users, and will not be saved. Rendering twice shows an error on the console about a negative user count.

The simplest fix is just to remove the freeing of all the materials. This works, and when Blender quits it doesn't complain of any memory leaks. I'll do some more tests with ASAN to check whether this is really the right way to go.

diff --git a/source/blender/freestyle/intern/blender_interface/BlenderStrokeRenderer.cpp b/source/blender/freestyle/intern/blender_interface/BlenderStrokeRenderer.cpp
index c88d5f24b5d..22718c29745 100644
--- a/source/blender/freestyle/intern/blender_interface/BlenderStrokeRenderer.cpp
+++ b/source/blender/freestyle/intern/blender_interface/BlenderStrokeRenderer.cpp
@@ -200,15 +200,6 @@ BlenderStrokeRenderer::~BlenderStrokeRenderer()
     }
   }
 
-  // release materials
-  Link *lnk = (Link *)freestyle_bmain->materials.first;
-
-  while (lnk) {
-    Material *ma = (Material *)lnk;
-    lnk = lnk->next;
-    BKE_id_free(freestyle_bmain, ma);
-  }
-
   BLI_ghash_free(_nodetree_hash, NULL, NULL);
 
   DEG_graph_free(freestyle_depsgraph);

So I tried it again. I found a trick for this.
This time I turned on the fake user. When I reopened the file, the image was present.

Don't know if this will really help. But I think I'll just inform.

@Tamito Kajiyama (kjym3) @Shinsuke Irie (irie) what do you think about the fix I described in my previous comment? From what I can see it works well (I also tested on https://download.blender.org/demo/test/AtvBuggy.zip) and causes no crashes or warnings about memory leaks.

I did some more testing, and with or without my code, there are the same number of calls to material_free_data() and ntree_free_data(), strengthening my hypothesis that the removed code is unnecessary.

@Gurpreet Singh (machman_3d) Thanks for the problem report. Using fake users fully makes sense as a workaround, because the reported bug is caused by a wrong decrement of image data block user counts.

@Sybren A. Stüvel (sybren) Many thanks for looking into the issue. I confirmed that your proposed fix indeed resolved the problem and no side-effect was seen.

The code segment you proposed to remove was intended to release a material auto-generated by BlenderStrokeRenderer::GetStrokeShader() every time a Freestyle rendering process takes place.

Removing the code seems inappropriate because of a risk of memory leaks. I have no idea why no such leak is seen though.

I tried to address the reported issue by properly incrementing the user counts of image data blocks referenced from texture nodes in Freestyle line style node trees.

diff --git a/source/blender/freestyle/intern/blender_interface/BlenderStrokeRenderer.cpp b/source/blender/freestyle/intern/blender_interface/BlenderStrokeRenderer.cpp
index c88d5f24b5d..00898f2b5c4 100644
--- a/source/blender/freestyle/intern/blender_interface/BlenderStrokeRenderer.cpp
+++ b/source/blender/freestyle/intern/blender_interface/BlenderStrokeRenderer.cpp
@@ -252,6 +252,13 @@ Material *BlenderStrokeRenderer::GetStrokeShader(Main *bmain,
     // make a copy of linestyle->nodetree
     ntree = ntreeCopyTree_ex(iNodeTree, bmain, do_id_user);

+    // loop over nodes to increment the user count of image data blocks
+    for (bNode *node = (bNode *)ntree->nodes.first; node; node = node->next) {
+      if (ELEM(node->type, SH_NODE_TEX_IMAGE, SH_NODE_TEX_ENVIRONMENT)) {
+        id_us_plus(node->id);
+      }
+    }
+
     // find the active Output Line Style node
     for (bNode *node = (bNode *)ntree->nodes.first; node; node = node->next) {
       if (node->type == SH_NODE_OUTPUT_LINESTYLE && (node->flag & NODE_DO_OUTPUT)) {

I have a few questions with respect to my solution:

  • Is maintaining ID reference counts in this way the right thing to do? Is there a better way to achieve the same effect?
  • Are there any other ID data blocks referenced from material nodes?

If you or some other main developers could answer these questions, I would be greatly appreciate it.

Temporary datablocks used for rendering should not increase user counts on datablocks that are actually part of the .blend file, or modify them in any way that might be visible in the user interface or Python API.

This is all more complicated than it should be though. There is no reason for the freestyle_bmain to exist beyond the lifetime of BlenderStrokeRenderer, that's a leftover from Blender Internal. We can just free the entire thing and not bother trying to manually free particular datablocks.

Awesome. You guys are the best.