Fix #115183: Show errors only shows errors when channels have been expanded #116815

Christoph Lendenfeld · 2024-01-05T09:51:34+01:00

2024-01-05 09:51:34 +01:00

When enabling the "Only Show Errors" option in the Dope Sheet/Graph Editor,
it might not show anything even though there are FCurves with errors.
The issue is that the flag FCURVE_DISABLED is set by the drawing code,
which means the channels have to be visible first.
This creates a workflow issue where you want to filter down to only the
curves with problems, but you need to find them first...

This PR fixes the issue by validating all FCurves whenever the option
is enabled.

When enabling the "Only Show Errors" option in the Dope Sheet/Graph Editor, it might not show anything even though there are FCurves with errors. The issue is that the flag `FCURVE_DISABLED` is set by the drawing code, which means the channels have to be visible first. This creates a workflow issue where you want to filter down to only the curves with problems, but you need to find them first... This PR fixes the issue by validating all FCurves whenever the option is enabled.

Christoph Lendenfeld added the

Module

Animation & Rigging

label 2024-01-05 09:51:34 +01:00

Christoph Lendenfeld added 2 commits 2024-01-05 09:51:43 +01:00

e78c478bf3 the fix

3c9d763f7c Merge branch 'main' into fix_invalid_fcu

Sybren A. Stüvel requested review from Sybren A. Stüvel 2024-01-09 14:41:20 +01:00

Sybren A. Stüvel reviewed 2024-01-09 15:09:18 +01:00

Sybren A. Stüvel left a comment

This is quite a severe degredation, however that function is only run
if the "Only Show Errors" option is enabled. So it might be a valid tradeoff.

I agree it's quite a degradation, but still, with 10k curves that would sum up to 2.8ms. I think that's certainly acceptable in cases when you're problem hunting.

Ideally we'd validate all FCurves once whenever the button is enabled,
but I don't know of a way to properly do that (we need bContext for the filtering code)

I think that's certainly worth a try. Check the declaration of the show_only_errors property in RNA (source/blender/makesrna/intern/rna_action.cc). You can declare an on-update callback using RNA_def_property_update() (the parameter that's currently a nullptr is actually an optional function name string). Set the PROP_CONTEXT_UPDATE flag (RNA_def_property_flag(prop, PROP_CONTEXT_UPDATE);) so the RNA system knows you want a bContext* parameter. Search for "rna_Light_use_nodes_update" if you want to see an example.

> This is quite a severe degredation, however that function is only run > if the "Only Show Errors" option is enabled. So it might be a valid tradeoff. I agree it's quite a degradation, but still, with 10k curves that would sum up to 2.8ms. I think that's certainly acceptable in cases when you're problem hunting. > Ideally we'd validate all FCurves once whenever the button is enabled, > but I don't know of a way to properly do that (we need `bContext` for the filtering code) I think that's certainly worth a try. Check the declaration of the `show_only_errors` property in RNA (`source/blender/makesrna/intern/rna_action.cc`). You can declare an on-update callback using `RNA_def_property_update()` (the parameter that's currently a `nullptr` is actually an optional function name string). Set the `PROP_CONTEXT_UPDATE` flag (`RNA_def_property_flag(prop, PROP_CONTEXT_UPDATE);`) so the RNA system knows you want a `bContext*` parameter. Search for `"rna_Light_use_nodes_update"` if you want to see an example.

Christoph Lendenfeld added 1 commit 2024-01-09 15:33:08 +01:00

05a807dd9f Merge branch 'main' into fix_invalid_fcu

Christoph Lendenfeld added 1 commit 2024-01-09 16:44:24 +01:00

1585091ef9 only run validation when button is enabled

Christoph Lendenfeld added 2 commits 2024-01-09 16:51:06 +01:00

20c1599cb7 simplify code

532ef329cd fix flag check

Christoph Lendenfeld commented

2024-01-09 16:52:08 +01:00

@dr.sybren thanks for the hint. I was able to add the callback function when the flag is enabled!

Sybren A. Stüvel approved these changes 2024-01-16 18:00:06 +01:00

Sybren A. Stüvel left a comment

LGTM!

Just one thought that you may want to test out before landing: if the error state needs updating because it's out of date, it might also need clearing? I've left an inline comment. Please check if such an addition is necessary, if not this can land as-is as it does fix the bug it's intending to fix.

LGTM! Just one thought that you may want to test out before landing: if the error state needs updating because it's out of date, it might also need clearing? I've left an inline comment. Please check if such an addition is necessary, if not this can land as-is as it does fix the bug it's intending to fix.

source/blender/makesrna/intern/rna_action.cc Outdated

						
				@ -351,0 +378,4 @@

				    PointerRNA id_ptr = RNA_id_pointer_create(ale->id);

				    if (!RNA_path_resolve_property(&id_ptr, fcu->rna_path, &ptr, &prop)) {

				      fcu->flag |= FCURVE_DISABLED;

				    }

Sybren A. Stüvel commented

2024-01-16 17:57:52 +01:00

I don't quite have an example blend file, but my gut feeling says this should properly take control over the FCURVE_DISABLED flag, and thus also be able to clear it:

    if (RNA_path_resolve_property(&id_ptr, fcu->rna_path, &ptr, &prop)) {
      fcu->flag &= ~FCURVE_DISABLED;
    }
    else {
      fcu->flag |= FCURVE_DISABLED;
    }

I don't quite have an example blend file, but my gut feeling says this should properly take control over the `FCURVE_DISABLED` flag, and thus also be able to clear it: ```cpp if (RNA_path_resolve_property(&id_ptr, fcu->rna_path, &ptr, &prop)) { fcu->flag &= ~FCURVE_DISABLED; } else { fcu->flag |= FCURVE_DISABLED; } ```

Christoph Lendenfeld commented

2024-01-16 18:29:33 +01:00

I couldn't think of a case where this would matter, but I added it anyway because I feel like its the more correct solution

👍 1

dr.sybren marked this conversation as resolved

Christoph Lendenfeld added 2 commits 2024-01-16 18:28:28 +01:00

72c61d67d5 Merge branch 'main' into fix_invalid_fcu

bbd709344c remove flag if path is resolved

Christoph Lendenfeld merged commit d08e826b24 into main

2024-01-18 10:27:23 +01:00

Christoph Lendenfeld deleted branch fix_invalid_fcu

2024-01-18 10:27:28 +01:00