Main Changes:
- Refactor of what happens when ctrl+shift clicking on a node to link to a viewer. The behavior of the geometry nodes viewer is a bit more complex than the compositor viewer.
- Evaluation, display and caching of fields in the spreadsheet.
Details
Details
Diff Detail
Diff Detail
- Repository
- rB Blender
Event Timeline
Comment Actions
- Merge branch 'master' into temp-field-spreadsheet
- improve link behavior in node editor
- add caching for spreadsheet
Comment Actions
Overall this looks good. It's nice that you went a bit further to generalize things that will be helpful in the future.
I think this needs versioning, since it adds struct to the viewer node storage. Or it could just use node->custom1, I'd be fine with that too (but I guess it would still need versioning).
I guess the caching can be used for the BMesh -> Mesh conversion we currently do on every redraw too!
| source/blender/editors/space_spreadsheet/spreadsheet_data_source.hh | ||
|---|---|---|
| 40 | as first -> as the first | |
| source/blender/editors/space_spreadsheet/spreadsheet_data_source_geometry.cc | ||
| 128 | I haven't figured out what happened with the default column width handling, or why that's related to the patch, what am I missing? | |
| 153 | I guess this means attributes with the name "Viewer" won't be visible in that mode of the spreadsheet. Not really a problem, just a bit funny to note. | |
| 625 | This stores evaluated fields, but is called GeometryComponentCache. I guess you thought of adding more to this? Or maybe just a comment like Stores the result of fields evaluated on a geometry component would clarify the purpose a bit. | |
| 627 | This could use a comment with some explanation. And yay, GArray is helpful! | |
| source/blender/editors/space_spreadsheet/spreadsheet_data_source_geometry.hh | ||
| 31 | Do you expect this to be used for something else soon too? Otherwise, ViewerNodeColumns would be a bit more to the point. | |
| 33 | Maybe add a comment here mentioning what the key and value mean in this map. Particularly that the span references data stored in the spreadsheet cache. | |
| source/blender/editors/space_spreadsheet/spreadsheet_data_source_geometry.cc | ||
|---|---|---|
| 128 | I moved this to a more central place in space_spreadsheet.cc. We didn't do it before because we didn't have SPREADSHEET_VALUE_TYPE_*. | |
| 153 | True.. Could be worked around in the future by adding more info to column_id. | |
Comment Actions
I guess the caching can be used for the BMesh -> Mesh conversion we currently do on every redraw too!
Yes, I think that should work.
Comment Actions
- don't show columns when there are no elements
- socket socket identifiers
- Merge branch 'master' into temp-field-spreadsheet
| source/blender/editors/space_node/node_relationships.cc | ||
|---|---|---|
| 642–657 | I'm not sure I have an answer at the moment, but we should think of a better place to put this and custom_data_type_to_socket_type | |
| source/blender/editors/space_spreadsheet/spreadsheet_cache.cc | ||
| 70–73 | Why not do this? Am I missing something? for (const int i : keys_.index_range()) {
if (keys_[i]->is_used) {
continue;
}
keys_.remove_and_reorder(i);
}And I guess it seems like the first loop in this function could use a range-based for loop too. | |
| source/blender/makesrna/intern/rna_nodetree.c | ||
| 11089 | I'd just remove the tooltip here, it's not very helpful and it's sort of conceptually incorrect. | |
| source/blender/editors/space_node/node_relationships.cc | ||
|---|---|---|
| 642–657 | Yeah, agreed, can be moved elsewhere later. | |
| source/blender/editors/space_spreadsheet/spreadsheet_cache.cc | ||
| 70–73 |
This doesn't work when you remove elements while iterating over the vector.
Then I wouldn't have access to the iterator which I need to use the remove function that allows removing during iteration. | |
| source/blender/editors/space_spreadsheet/spreadsheet_cache.cc | ||
|---|---|---|
| 70–73 | Okay, thanks for the explanation. | |