Page MenuHome

Fix T40315: Boolean modifier with Freestyle edges
AbandonedPublic

Authored by Tamito Kajiyama (kjym3) on May 28 2014, 4:50 PM.

Details

Summary

Attempts to fix T40315: Boolean modifier with Freestyle edges.

Two issues:

  1. make_freestyle_edge_mark_hash() in convertblender.c was referring to the original mesh of a derived mesh to determine Freestyle edge marks in the derived mesh (patch contribution by Sergey).
  2. get_dm_for_modifier() in MOD_util.c was not returning a proper derived mesh for render (was a future to-do).

The first issue had to be fixed to keep Freestyle edge marks originating from the parameter object of the boolean modifier. The other issue was causing missing edge marks from the parameter object also modified by a modifier stack of its own.

Diff Detail

Event Timeline

Sergey Sharybin (sergey) requested changes to this revision.May 28 2014, 5:19 PM

Change in pipeline.c seems ok to me (didn't test it, but assume it's the same as in original patch).

Changes int get_dm_for_modifier() are wrong. Modifier stack should never touch other's object for write. Can't say more detils atm because not sure which exact issue you're trying to fix here.

The proposed changes in get_dm_for_modifier() are necessary to get a CD_FREESTYLE_EDGE layer populated properly in the derived mesh from the object specified in the boolean modifier parameter.

Below please find attached an example .blend file to illustrate the problem:

The expected render is shown below:

The sphere in the back is the modifier parameter object. It's a cube mesh with a subsurf modifier. Without the proposed changes, the derived mesh from the original cube does not have a CD_FREESTYLE_EDGE layer for some unknown reason (my wild guess is because of a derived mesh for view) and thus the edge marks (in red) won't appear.

My guess is that CD_MASK_FREESTYLE_EDGE | CD_MASK_FREESTYLE_FACE are to be added to CD_MASK_BAREMESH.

Another idea would be to use existing tag_dependend_objects_for_render() (which probably shouldn't be modified or maybe it need to check for missing CD_MASK_FREESTYLE_* and tag objects) and make it so BKE_object_handle_update_ex() adds freestyle masks to the mask if evaluation happens for render.

This is what i meant http://www.pasteall.org/51917/diff

Seems we're doing some unneeded object updates before rendering (need to check for derived_final != NULL and datamasks in tag_dependend_objects_for_render() but it's out of the scope of this patch actually. I'll look into it separately.

The diff http://www.pasteall.org/51917/diff seems not working correctly as the object specified as the boolean modifier parameter is rendered in the preview subsurf level (2 in my example scene) instead of the render level (3). That is because makeDerivedMesh() calls mesh_calc_modifiers() with useRenderParams set to false. Passing eval_ctx->for_render to mesh_calc_modifiers() would not work either -- after rendering, the viewport shows meshes in the render subsurf level.

Please note that in my proposed patch, the mesh_create_derived_render() function (called during the modifier application) does not modify the passed object AFAIK. The evaluation of the boolean modifier will keep everything in the modifier parameter object unchanged. Even so, are the proposed changes still considered wrong?

Another idea is to call mesh_create_derived_render() from mesh_calc_modifiers() in advance of the call of the boolean modifier's applyModifier(), possibly by means of modifiers_foreachObjectLink(). I am not sure if that will end up in the viewport showing meshes in the render subsurf level after rendering.

the object specified as the boolean modifier parameter is rendered in the preview subsurf level

This is a known limitation which has nothing to do with the original report.

Issues with your approach:

  • mesh_create_derived_render() _might_ modify object -- caches, duplicators etc.
  • It'll re-create derived render for every boolean modifier which shares the same object as a right operand leading to quite huge overhead.

Now, mesh_calc_modifiers() should not create both preview and render DM. Ideally it should create either one or another. In my GSoC branch i had an idea of having ob->derivedRender but it turned a real headache to implement properly because of the way how BI duplication works.

I would suggest fixing freestyle-related issues in this patch and leave bigger issues for later. If they were so trivial to solve as the fix you're suggesting it would have already been fixed when merging my GSoC branch.

Ok, thank you for taking time for the code review.

Just saying, there was already a very similar problem about the freestyle edges with the subsurf modifier recently (patched by @Campbell Barton (campbellbarton) : rB3182c54da6fd). I think now the subsurf modifier creates a copy of the edge customdata ONLY if its apply hook gets the MOD_APPLY_USECACHE flag (I think when the user presses the apply button). Perhaps now it supposed to provide this always because you are not referencing the data through ORIGINDEX anymore?

Maybe the problem is that the subsurf modifier is not working correctly in this new case?

@Fazekas Laszlo (totoro),
As far as I tested, the subsurf modifier gives correct edge data regardless of the MOD_APPLY_USECACHE flag.