Page MenuHome

Property Search: All tabs
ClosedPublic

Authored by Hans Goudey (HooglyBoogly) on Sep 9 2020, 8:46 PM.

Details

Summary

This patch enables property search for all tabs in the property editor.
This is definitely the most "crazy" of the changes in terms of code, but
it's also where the new functionality gets good.

In order to make interaction faster, if the editor's current tab doesn't
have a result, the search moves you to the next panel that does.
That way you can just press ctrl-F, search, and have the result appear.

In order to keep the searching safe, to make sure the editor isn't influenced
by anything that happens while building the layout for the other tabs,
the space is duplicated and the new search is run in the duplicated
editor. This also helps isolate this code, which could be fairly invasive
otherwise. While we could run the regular "single tab" property search,
that would also do everything else related to the layout pass, which less
efficient, and probably more complicated to maintain down the line.
So this patch implements basic searching panel searching code.

The search match status for every current tab of the property editor is
stored in a runtime field and them displayed later by dimming icons in
the tab selector panel to the left. The functionality for that dimming is
in D8858.

Note that the tool settings tab works slightly different than the other
tabs, so I've disabled searching it for this patch. That would be a relatively
simple improvement, but would just require a bit of refactoring of
existing code.

Future Improvements
This patch does not address the performance aspects of searching every
tab. This shouldn't be too bad, but it would be good to make some changes
to improve this, ideally in separate patches. I have done some initial profiling,
and it looks like a significant portion of time is spent doing string comparisons,
so that may be the best place to start. Here are some ideas:

  1. Use ghash instead of string lookups for panel types
  2. Possibly only search in other tabs while editing search string. I would like to avoid this though.
  3. Look into using ED_region_tag_redraw_no_rebuild for some interactions like panel dragging.
  4. Exit early when a search match is found for a tab. Done

Diff Detail

Repository
rB Blender
Branch
property-search-all-tabs-v2 (branched from master)
Build Status
Buildable 10196
Build 10196: arc lint + arc unit

Event Timeline

Hans Goudey (HooglyBoogly) requested review of this revision.Sep 9 2020, 8:46 PM
Hans Goudey (HooglyBoogly) edited the summary of this revision. (Show Details)
Campbell Barton (campbellbarton) requested changes to this revision.Sep 10 2020, 1:07 PM
Campbell Barton (campbellbarton) added inline comments.
source/blender/editors/interface/interface_panel.c
2678

Since this was previously not NULL, a short comment for why this can be NULL would be good.

source/blender/makesdna/DNA_space_types.h
162–164

This can go in a runtime struct SpaceProperties_Runtime.

This revision now requires changes to proceed.Sep 10 2020, 1:07 PM
  • Property Search: New implementation for all-tab search
  • Property Search: Fixes for all tab search V2
  • Cleanup: Remove unecessary whitespace change
  • Merge branch master after splitting off some cleanup changes
  • Merge branch 'property-search-highlight-tabs' into property-search-all-tabs-v2
Hans Goudey (HooglyBoogly) marked 2 inline comments as done.
  • Cleanup: Fix clang tidy else after return
  • Property Search: Use local variables instead of heap variables
  • Property Search: Set the area's spacedata to the duplicate

I'm still chasing two bugs. One is a weird memory leak of buttons that I haven't been able
to reproduce. The other is a consistent memory leak of the SpaceProperties->runtime struct
which happens on every quit, even though it is freed in buttons_free. I find this second
one a bit baffling right now.

The other is a consistent memory leak of the SpaceProperties->runtime struct
which happens on every quit, even though it is freed in buttons_free. I find this second
one a bit baffling right now.

D8856#218164 should explain the memory leak.

One thing I noticed: Searching in other tabs causes the material preview icons to be generated in the background. Would be nice to avoid.

  • Merge branch 'master' into property-search-all-tabs-v2
  • Merge branch 'master' into property-search-all-tabs-v2
  • Use a boolean array and bitmap to store highlight instead of int
  • Remove unecessary freeing of panel list

@Julian Eisel (Severin) This should be ready to go now. I looked into the preview issue. I think that issue might have been related to something else I've fixed.
I've placed a breakpoint in ED_preview_draw and ED_preview_shader_job and they only ran when one of those panels was open and visible. I'm curious how you tested it initially.

LGTM, suggest comment.

source/blender/editors/space_buttons/space_buttons.c
298

This logic is quite strange/exceptional - that it copies the UI end generates a layout.

I think this could use a doxygen group with a title such as Off Screen Layout Generation for Search (or something to this effect).

Julian Eisel (Severin) added inline comments.
source/blender/makesrna/intern/rna_space.c
4499–4501

You only need to set one member, and the rest or the memory is guaranteed to be 0'ed, afaik (defined in the C standard).

Hans Goudey (HooglyBoogly) marked an inline comment as done.Oct 13 2020, 7:07 PM
Hans Goudey (HooglyBoogly) added inline comments.
source/blender/editors/space_buttons/space_buttons.c
298

There are no doxygen sections in this file yet, so I'll do that in a follow-up commit.

We could also consider moving this logic to a separate file, like buttons_property_search.c

This revision was not accepted when it landed; it landed in state Needs Review.Oct 13 2020, 8:11 PM
This revision was automatically updated to reflect the committed changes.