More flexible callback for persistent panel expand flags #108648
No reviewers
Labels
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset Browser
Interest
Asset Browser Project Overview
Interest
Audio
Interest
Automated Testing
Interest
Blender Asset Bundle
Interest
BlendFile
Interest
Collada
Interest
Compatibility
Interest
Compositing
Interest
Core
Interest
Cycles
Interest
Dependency Graph
Interest
Development Management
Interest
EEVEE
Interest
EEVEE & Viewport
Interest
Freestyle
Interest
Geometry Nodes
Interest
Grease Pencil
Interest
ID Management
Interest
Images & Movies
Interest
Import Export
Interest
Line Art
Interest
Masking
Interest
Metal
Interest
Modeling
Interest
Modifiers
Interest
Motion Tracking
Interest
Nodes & Physics
Interest
OpenGL
Interest
Overlay
Interest
Overrides
Interest
Performance
Interest
Physics
Interest
Pipeline, Assets & IO
Interest
Platforms, Builds & Tests
Interest
Python API
Interest
Render & Cycles
Interest
Render Pipeline
Interest
Sculpt, Paint & Texture
Interest
Text Editor
Interest
Translations
Interest
Triaging
Interest
Undo
Interest
USD
Interest
User Interface
Interest
UV Editing
Interest
VFX & Video
Interest
Video Sequencer
Interest
Virtual Reality
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Interest: X11
Legacy
Blender 2.8 Project
Legacy
Milestone 1: Basic, Local Asset Browser
Legacy
OpenGL Error
Meta
Good First Issue
Meta
Papercut
Meta
Retrospective
Meta
Security
Module
Animation & Rigging
Module
Core
Module
Development Management
Module
EEVEE & Viewport
Module
Grease Pencil
Module
Modeling
Module
Nodes & Physics
Module
Pipeline, Assets & IO
Module
Platforms, Builds & Tests
Module
Python API
Module
Render & Cycles
Module
Sculpt, Paint & Texture
Module
Triaging
Module
User Interface
Module
VFX & Video
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Priority
High
Priority
Low
Priority
Normal
Priority
Unbreak Now!
Status
Archived
Status
Confirmed
Status
Duplicate
Status
Needs Info from Developers
Status
Needs Information from User
Status
Needs Triage
Status
Resolved
Type
Bug
Type
Design
Type
Known Issue
Type
Patch
Type
Report
Type
To Do
No Milestone
No project
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: blender/blender#108648
Loading…
Reference in New Issue
No description provided.
Delete Branch "LukasTonne/blender:panel_expand_flags_bit_vector"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
"Expand flags" are persistent storage that helps Blender remember the open/closed state of panels, even when the panels themselves are destroyed (e.g. on workspace changes). These flags are stored in various places where panels are created (e.g. modifiers) and currently exposed to the panel system via get/set callback.
So far the callback supports up to 16 bits by returning a
short
by value. This is fine for existing fixed panel layouts, but for future dynamic panels (node modifier socket sub-panels) this is a significant limitation. To make more flag bits possible the callback now supports returning an arbitrary array of bits as auint64_t *
.This simplifies the internal code too, because we can just use a BitIterator to loop over bits in sequence instead of using offsets and masks. It's the responsibility of the panel creating code to ensure there are enough bits for the number of child panels generated. For now all panel types return a pointer to a
short
as before, but now a larger type could be used, including a fullBitVector
if needed.Lukas Tönne referenced this pull request2023-06-06 10:41:35 +02:00
Mostly looks good! I think returning the integer via pointer doesn't really get us anything at the moment-- it still gives a 64 panel limit. That's fine for now I guess, so we might as well return a
uint64_t
directly.In the future it would be nice to return
BitVector
, or an allocated array ofuint64_t
.@ -289,15 +289,9 @@ typedef struct PanelType {
* data item. Called on draw updates.
* \note Sub-panels are indexed in depth first order,
* the visual order you would see if all panels were expanded.
* \return Pointer to bit array with at least one bit for the panel and for each child panel.
at least one bit
is a bit confusing. I think it means "the array is big enough, don't worry!" but it sounds like some panels might get two bits for some reason.Yes, it's in preparation for returning a larger bit vector for modifier panels. I used
uint64_t
because it's the storage type used forBitVector
/BitIterator
although it's quite arbitrary, could even returnvoid *
. The main difference is that we expect this to be a contiguous array, so the iterator can go beyond the initial integer.BitIterator
takes care of this.Agreed. But this is still a C callback. Having the additional size property would give us a way to assert that there are enough bits for all the panels. So far we just assume that the panel creator code knows to reserve enough bits.
Sure, I'm just not sure that returning a pointer instead of a integer value helps prepare for
BitVector
more. Maybe until we can make this a C++ callback, it's better to use the value? It's also just a bit weird to have auint64_t
pointer that points to ashort
.Would a
void *
be better? We don't really care about the underlying data type. Returning avoid *
at least does not suggest that the data has any specific size (other than "large enough for all panels").Returning bits by value would also require a setter function again, at which point PR would be pointless. Or is it pointerless?
Could also return pointer + size, and then we construct a
BitSpan
locally in interface_panel.cc. Once PanelType supports C++ we can swap this out for a BitSpan in the callback itself.Either way, without split
get
andset
callbacks,BitVector
doesn't really work at all for the future, I think that's why I was confused.I don't think
void *
would be better, it just removes all type information. If the situation feels better to you I'm fine with this change, but including the downsides, this doesn't really feel like an improvement to me personally. I guess it's better if I think about it as a first step to usingBitSpan
.Ok, i'm dropping it then, can revisit when we can actually do this in C++.
Pull request closed