From 4d958ca19ad83ca0d8dcdda845aff5498b25485e Mon Sep 17 00:00:00 2001 From: Christoph Lendenfeld Date: Thu, 7 Mar 2024 17:15:02 +0100 Subject: [PATCH 1/2] Fix: View in Graph Editor not working when object is unselected The new operator "View in Graph Editor" (#114407) only looked at the selection when generating the FCurve bounds. This failed in certain cases if things are not selected, or not selectable. The simplest example is to key the emission strength of the world settings and try to view that. This was mentioned by Jonathan Lampel on the PR #114407 The fix is to also resolve the bounds of the animation data on the button. This means for a few FCurves the bounds might be calculated twice but it doesn't change the overall result. This has another side effect though: The Graph Editor might zoom into an area where there is no FCurve visible because the thing is not selected so it isn't shown. This issue existed before if filters were applied, but is now more pronounced since by default the Graph Editor doesn't show unselected objects. A warning is raised when at least one of the FCurves that are viewed is not visible with the current filter settings. The view will still zoom into that area. Pull Request: https://projects.blender.org/blender/blender/pulls/118658 --- .../editors/animation/anim_channels_edit.cc | 217 +++++++++++++----- 1 file changed, 155 insertions(+), 62 deletions(-) diff --git a/source/blender/editors/animation/anim_channels_edit.cc b/source/blender/editors/animation/anim_channels_edit.cc index 3c968f19df8..4cc9dedf9bc 100644 --- a/source/blender/editors/animation/anim_channels_edit.cc +++ b/source/blender/editors/animation/anim_channels_edit.cc @@ -4532,13 +4532,70 @@ static void deselect_all_fcurves(bAnimContext *ac, const bool hide) ANIM_animdata_freelist(&anim_data); } -static rctf calculate_selection_fcurve_bounds_and_unhide( - bContext *C, - ListBase /* CollectionPointerLink */ *selection, - PropertyRNA *prop, - const blender::StringRefNull id_to_prop_path, - const int index, - const bool whole_array) +static int count_fcurves_hidden_by_filter(bAnimContext *ac, const blender::Span fcurves) +{ + ListBase anim_data = {nullptr, nullptr}; + if (ac->sl->spacetype != SPACE_GRAPH) { + return 0; + } + SpaceGraph *sipo = (SpaceGraph *)ac->sl; + const eAnimFilter_Flags filter = eAnimFilter_Flags(sipo->ads->filterflag); + ANIM_animdata_filter(ac, &anim_data, filter, ac->data, eAnimCont_Types(ac->datatype)); + + /* Adding FCurves to a map for quicker lookup times. */ + blender::Map filtered_fcurves; + LISTBASE_FOREACH (bAnimListElem *, ale, &anim_data) { + FCurve *fcu = (FCurve *)ale->key_data; + filtered_fcurves.add(fcu, true); + } + + int hidden_fcurve_count = fcurves.size(); + for (FCurve *fcu : fcurves) { + if (filtered_fcurves.contains(fcu)) { + hidden_fcurve_count--; + } + } + ANIM_animdata_freelist(&anim_data); + return hidden_fcurve_count; +} + +static blender::Vector get_fcurves_of_property( + ID *id, PointerRNA *ptr, PropertyRNA *prop, const bool whole_array, const int index) +{ + using namespace blender; + + AnimData *anim_data = BKE_animdata_from_id(id); + if (anim_data == nullptr) { + return Vector(); + } + + const std::optional path = RNA_path_from_ID_to_property(ptr, prop); + + blender::Vector fcurves; + if (RNA_property_array_check(prop) && whole_array) { + const int length = RNA_property_array_length(ptr, prop); + for (int i = 0; i < length; i++) { + FCurve *fcurve = BKE_animadata_fcurve_find_by_rna_path( + anim_data, path->c_str(), i, nullptr, nullptr); + if (fcurve != nullptr) { + fcurves.append(fcurve); + } + } + } + else { + FCurve *fcurve = BKE_animadata_fcurve_find_by_rna_path( + anim_data, path->c_str(), index, nullptr, nullptr); + if (fcurve != nullptr) { + fcurves.append(fcurve); + } + } + return fcurves; +} + +static rctf calculate_fcurve_bounds_and_unhide(SpaceLink *space_link, + Scene *scene, + ID *id, + const blender::Span fcurves) { rctf bounds; bounds.xmin = INFINITY; @@ -4546,16 +4603,59 @@ static rctf calculate_selection_fcurve_bounds_and_unhide( bounds.ymin = INFINITY; bounds.ymax = -INFINITY; - SpaceLink *ge_space_link = CTX_wm_space_data(C); - if (ge_space_link->spacetype != SPACE_GRAPH) { + if (space_link->spacetype != SPACE_GRAPH) { + return bounds; + } + + AnimData *anim_data = BKE_animdata_from_id(id); + if (anim_data == nullptr) { return bounds; } - Scene *scene = CTX_data_scene(C); float frame_range[2]; get_view_range(scene, true, frame_range); + float mapped_frame_range[2]; + mapped_frame_range[0] = BKE_nla_tweakedit_remap( + anim_data, frame_range[0], NLATIME_CONVERT_UNMAP); + mapped_frame_range[1] = BKE_nla_tweakedit_remap( + anim_data, frame_range[1], NLATIME_CONVERT_UNMAP); + const bool include_handles = false; + for (FCurve *fcurve : fcurves) { + fcurve->flag |= (FCURVE_SELECTED | FCURVE_VISIBLE); + rctf fcu_bounds; + get_normalized_fcurve_bounds(fcurve, + anim_data, + space_link, + scene, + id, + include_handles, + mapped_frame_range, + &fcu_bounds); + + if (BLI_rctf_is_valid(&fcu_bounds)) { + BLI_rctf_union(&bounds, &fcu_bounds); + } + } + + return bounds; +} + +static rctf calculate_selection_fcurve_bounds(bAnimContext *ac, + ListBase /* CollectionPointerLink */ *selection, + PropertyRNA *prop, + const blender::StringRefNull id_to_prop_path, + const int index, + const bool whole_array, + int *r_filtered_fcurve_count) +{ + rctf bounds; + bounds.xmin = INFINITY; + bounds.xmax = -INFINITY; + bounds.ymin = INFINITY; + bounds.ymax = -INFINITY; + LISTBASE_FOREACH (CollectionPointerLink *, selected, selection) { ID *selected_id = selected->ptr.owner_id; if (!BKE_animdata_id_is_animated(selected_id)) { @@ -4574,45 +4674,11 @@ static rctf calculate_selection_fcurve_bounds_and_unhide( resolved_ptr = selected->ptr; resolved_prop = prop; } - const std::optional path = RNA_path_from_ID_to_property(&resolved_ptr, - resolved_prop); - - AnimData *anim_data = BKE_animdata_from_id(selected_id); - blender::Vector fcurves; - if (RNA_property_array_check(prop) && whole_array) { - const int length = RNA_property_array_length(&selected->ptr, prop); - for (int i = 0; i < length; i++) { - FCurve *fcurve = BKE_animadata_fcurve_find_by_rna_path( - anim_data, path->c_str(), i, nullptr, nullptr); - if (fcurve != nullptr) { - fcurves.append(fcurve); - } - } - } - else { - FCurve *fcurve = BKE_animadata_fcurve_find_by_rna_path( - anim_data, path->c_str(), index, nullptr, nullptr); - if (fcurve != nullptr) { - fcurves.append(fcurve); - } - } - - for (FCurve *fcurve : fcurves) { - fcurve->flag |= (FCURVE_SELECTED | FCURVE_VISIBLE); - rctf fcu_bounds; - float mapped_frame_range[2]; - mapped_frame_range[0] = BKE_nla_tweakedit_remap( - anim_data, frame_range[0], NLATIME_CONVERT_UNMAP); - mapped_frame_range[1] = BKE_nla_tweakedit_remap( - anim_data, frame_range[1], NLATIME_CONVERT_UNMAP); - get_normalized_fcurve_bounds(fcurve, - anim_data, - ge_space_link, - scene, - selected_id, - include_handles, - mapped_frame_range, - &fcu_bounds); + blender::Vector fcurves = get_fcurves_of_property( + selected_id, &resolved_ptr, resolved_prop, whole_array, index); + *r_filtered_fcurve_count += count_fcurves_hidden_by_filter(ac, fcurves); + rctf fcu_bounds = calculate_fcurve_bounds_and_unhide(ac->sl, ac->scene, selected_id, fcurves); + if (BLI_rctf_is_valid(&fcu_bounds)) { BLI_rctf_union(&bounds, &fcu_bounds); } } @@ -4622,12 +4688,12 @@ static rctf calculate_selection_fcurve_bounds_and_unhide( static int view_curve_in_graph_editor_exec(bContext *C, wmOperator *op) { - PointerRNA ptr = {nullptr}; - PropertyRNA *prop = nullptr; + PointerRNA button_ptr = {nullptr}; + PropertyRNA *button_prop = nullptr; uiBut *but; int index; - if (!(but = UI_context_active_but_prop_get(C, &ptr, &prop, &index))) { + if (!(but = UI_context_active_but_prop_get(C, &button_ptr, &button_prop, &index))) { /* Pass event on if no active button found. */ return (OPERATOR_CANCELLED | OPERATOR_PASS_THROUGH); } @@ -4645,14 +4711,10 @@ static int view_curve_in_graph_editor_exec(bContext *C, wmOperator *op) bool path_from_id; std::optional id_to_prop_path; const bool selected_list_success = UI_context_copy_to_selected_list( - C, &ptr, prop, &selection, &path_from_id, &id_to_prop_path); + C, &button_ptr, button_prop, &selection, &path_from_id, &id_to_prop_path); - if (BLI_listbase_is_empty(&selection) || !selected_list_success) { - WM_report(RPT_ERROR, "Nothing selected"); - retval = OPERATOR_CANCELLED; - } - else if (!context_find_graph_editor( - C, &wm_context_temp.win, &wm_context_temp.area, &wm_context_temp.region)) + if (!context_find_graph_editor( + C, &wm_context_temp.win, &wm_context_temp.area, &wm_context_temp.region)) { WM_report(RPT_WARNING, "No open Graph Editor window found"); retval = OPERATOR_CANCELLED; @@ -4674,12 +4736,43 @@ static int view_curve_in_graph_editor_exec(bContext *C, wmOperator *op) } else { const bool isolate = RNA_boolean_get(op->ptr, "isolate"); + /* The index can be less than 0 e.g. on color properties. */ + const bool whole_array = RNA_boolean_get(op->ptr, "all") || index < 0; + deselect_all_fcurves(&ac, isolate); - const bool whole_array = RNA_boolean_get(op->ptr, "all"); + rctf bounds; + bounds.xmin = INFINITY; + bounds.xmax = -INFINITY; + bounds.ymin = INFINITY; + bounds.ymax = -INFINITY; + int filtered_fcurve_count = 0; + if (selected_list_success && !BLI_listbase_is_empty(&selection)) { + rctf selection_bounds = calculate_selection_fcurve_bounds(&ac, + &selection, + button_prop, + id_to_prop_path.value_or(""), + index, + whole_array, + &filtered_fcurve_count); + if (BLI_rctf_is_valid(&selection_bounds)) { + BLI_rctf_union(&bounds, &selection_bounds); + } + } - rctf bounds = calculate_selection_fcurve_bounds_and_unhide( - C, &selection, prop, id_to_prop_path.value_or(""), index, whole_array); + /* The object to which the button belongs might not be selected, or selectable. */ + blender::Vector button_fcurves = get_fcurves_of_property( + button_ptr.owner_id, &button_ptr, button_prop, whole_array, index); + filtered_fcurve_count += count_fcurves_hidden_by_filter(&ac, button_fcurves); + rctf button_bounds = calculate_fcurve_bounds_and_unhide( + ac.sl, ac.scene, button_ptr.owner_id, button_fcurves); + if (BLI_rctf_is_valid(&button_bounds)) { + BLI_rctf_union(&bounds, &button_bounds); + } + + if (filtered_fcurve_count > 0) { + WM_report(RPT_WARNING, "One or more F-Curves are not visible due to filter settings"); + } if (!BLI_rctf_is_valid(&bounds)) { WM_report(RPT_ERROR, "F-Curves have no valid size"); From e0c6db81ea0eba39dd5fae71189f1480c7be65fb Mon Sep 17 00:00:00 2001 From: Brecht Van Lommel Date: Thu, 7 Mar 2024 19:26:31 +0100 Subject: [PATCH 2/2] Tests: Blacklist microfacet hair test on GPU, due to different curve shape --- tests/python/cycles_render_tests.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/python/cycles_render_tests.py b/tests/python/cycles_render_tests.py index 043c9494c21..d7b65d94c19 100644 --- a/tests/python/cycles_render_tests.py +++ b/tests/python/cycles_render_tests.py @@ -53,6 +53,7 @@ BLACKLIST_GPU = [ "hair_transmission.blend", 'principled_hair_.*.blend', 'transparent_shadow_hair.*.blend', + "microfacet_hair_orientation.blend", # Inconsistent handling of overlapping objects. "T41143.blend", "visibility_particles.blend",