D11307 D11308 D11309 all seem to contain the same changes BKE_gpencil_get_lineart_global_limits for example, I'm guessing this is not intentional?

In D11307#289483, @Sebastian Parborg (zeddb) wrote:

D11307 D11308 D11309 all seem to contain the same changes BKE_gpencil_get_lineart_global_limits for example, I'm guessing this is not intentional?

Well that's the minor conflict I'm talking about. Just preserve that part and you should be good. The commit history is kinda mixed so this is the most separated way of doing the patches :P

You have to be able to separate out the common code more than this.
These patches seem to contain the object loading changes as well.
So you have not based this on those changes, you have included them.

That these depend on other changes are fine, but only the relevant changes should be included in the patch.

I noticed some stuff from the "floating edges" patch in here. But I trust that you properly merge these patches into master, so no need to clean these up in this review.

I still have an issue with the current way the section is laid out.

That this picture for example:

Does this mean that the "Filter Face Mask" option is on, but it is baked or does it mean that the option works but we can't change it while cached is on?

The answer here is that it doesn't work at all. Which I don't feel that the grayed out checkbox conveys.

In D11307#300630, @Sebastian Parborg (zeddb) wrote:

I still have an issue with the current way the section is laid out.

That this picture for example:

Does this mean that the "Filter Face Mask" option is on, but it is baked or does it mean that the option works but we can't change it while cached is on?

The answer here is that it doesn't work at all. Which I don't feel that the grayed out checkbox conveys.

It is supposed to mean "Face mark filtering is done in the first one and this one is ineffective". How about manually remove that checkbox when cache is on? so it's more like other panels.

I guess you can remove the checkbox when caching is on, yes.

You should also rename the section to "Face Mask Filtering"

And "Filter Face Mask" implies that we are filtering face mask and not filtering USING face masks.

In D11307#300945, @Sebastian Parborg (zeddb) wrote:

I guess you can remove the checkbox when caching is on, yes.

You should also rename the section to "Face Mask Filtering"

And "Filter Face Mask" implies that we are filtering face mask and not filtering USING face masks.

That makes sense. I'll do it.

It's now looking like this. I think it's more clear now.