Page MenuHome

Before starting a texture bake, check surfaces are enabled in the viewlayer. (T80395)
AbandonedPublic

Authored by Martijn Versteegh (Baardaap) on Sep 24 2020, 10:50 PM.

Details

Summary

When surfaces are disabled in the viewlayer, cycles can't bake to them
and crashes when trying to render initialized triangle data.

This patch checks if surfaces are enabled before starting the bake and
reports an error if they aren't.

Diff Detail

Repository
rB Blender
Branch
fixT80395 (branched from master)
Build Status
Buildable 10393
Build 10393: arc lint + arc unit

Event Timeline

Martijn Versteegh (Baardaap) requested review of this revision.Sep 24 2020, 10:50 PM
Aaron Carlisle (Blendify) added inline comments.
source/blender/editors/object/object_bake_api.c
1350

Changed 'turned off' to 'disabled' as per Blendify's suggestion.

'disabled' is better indeed.

Martijn Versteegh (Baardaap) marked an inline comment as done.Sep 24 2020, 11:56 PM

If we were to go this way, I would move the code to bake_objects_check instead.

That said, I would like to hear from @Brecht Van Lommel (brecht) if we shouldn't actually turn this option on in Cycles when baking.

My rational is that this view layer option may not be supported by other render engines, and baking would fail. Users would need to switch to Cycles to turn this on again before rendering.

I had put it in bake_objects_check first, but as it';s not really an object property that didn't feel like the right place to put it.

Is bake_object_check cycles only? Otherwise I could maybe add a check if the engine is cycles? That feels a bit hackish as well, as the view_layer->layflag is not really cycles specific I think? I'll have a look at where the filtering happens exactly. I think it is on the cycles side, but I'm not 100% sure.

The really clean way to solve it (if it is indeed cycles who does the filtering of surfaces from the layflag setting) would be start the baking and let cycles cleanly fail with some error message instead of crash (i.e. fixing it on the cycles side). That would mean back to the drawing board, since I don't yet have a clear picture of the code flow on the cycles side. I Start to get more or less how it works with packing everything in 'textures' etc. but it's a bit hard to wrap my head around.

Catching it here is a lot simpler. But in the long run might indeed lead to confusion if other render engines(which ignore view_layer->layflag) are used for baking.

In any case I'll have a look if it's simple to catch on the cycles side. But I would be grateful if someone else would also give their thoughts on it.

Just enabling surfaces when baking (en disabling them again after starting the bake job) also is a good idea indeed. I think I like that best. It's obviously that the user wants to do that anyway as baking with surfaces disabled is nonsensical.

I think that would also lead to the most user friendly workflow as it doesn't force you to click to the view layer, open the filter panel, click the surfaces checkbox and do everything again in reverse after the bake, as you probably disabled them for a reason.

I think the other plan (ignoring disabled surfaces when baking) is much better from a usability point of view. Futhermore, I think this patch is faulty since I only handle bake_exec( ), and not bake_invoke( ).
I thought bake_invoke would call bake_exec eventually. And I also tested the patch and it *seemed* to work. But I must have done something wrong wile testing, because a breakpoint on bake_exec( ) is never hit when bake_invoke( ) is called. At least not before the crash already happened.

If people disagree, this patch can be resurrected and made to handle invoke as well, for now I abandon it and I have created a new patch which makes cycles ignore disabled surfaces when baking. ( D9015 )