Suggestion: Lazily initialize IDs

[Diggsey]

Managing widget IDs feels like unnecessary boilerplate: every time I want an array of widgets I have to zip the widget state with my IdList.

I should just be able to put the widget ID inside my state struct, but I can't do this because initializing the ID requires a reference to the generator.

I think this boilerplate could be avoided by reserving a "null" ID value that can be default-constructed, and having the set() method on widgets take a &mut Id as input. This method would check if the ID is "null" and initialize it using the ID generator from the UiCell if it is. The caller never has to do anything with the ID other than preserve it.

[Diggsey]

FWIW, I implemented this as an extension trait, and it is way easier to use this way...

use conrod_core::widget::Id;
use conrod_core::{UiCell, Widget};

pub trait WidgetExt: Widget {
    fn set_opt<'a, 'b>(self, id: &mut Option<Id>, ui_cell: &'a mut UiCell<'b>) -> Self::Event;
}

impl<T: Widget> WidgetExt for T {
    fn set_opt<'a, 'b>(
        self,
        maybe_id: &mut Option<Id>,
        ui_cell: &'a mut UiCell<'b>,
    ) -> Self::Event {
        let id = if let Some(id) = *maybe_id {
            id
        } else {
            let id = ui_cell.widget_id_generator().next();
            *maybe_id = Some(id);
            id
        };
        self.set(id, ui_cell)
    }
}

[alvinhochun]

If you do this, it gets a bit inconvenient when you need to refer to the widget by its ID, e.g. when positioning and parenting other widgets. TBH .zip()-ing the IDs with other states doesn't seem that annoying to me. I use .zip quite often in other code too when I have SoA (struct of array).

To me, the more annoying part is resizing the id::List. Due to how conrod is designed, you can't free the memory used by stale widgets and the IDs aren't automatically recycled, therefore I try to recycle them in my ID lists. However, the List::resize implementation currently drops excess IDs, which means I have to write my own boilerplate code to resize it only if I need more IDs than there already are. Putting the IDs in the other AoS (array of struct) structs would make this even harder.

[Diggsey]

If you do this, it gets a bit inconvenient when you need to refer to the widget by its ID, e.g. when positioning and parenting other widgets.

I think this is only an issue if you need to refer to the widget before drawing it, which doesn't seem too much of a burden since you can't have cycles when positioning or parenting anyway. Also, this doesn't prohibit you from initializing specific IDs early if you need to refer to them before drawing the component.

TBH .zip()-ing the IDs with other states doesn't seem that annoying to me. I use .zip quite often in other code too when I have SoA (struct of array).

There are two problems with this beyond the ergonomics of zip.

1. The IdList only works if you only add and remove components to/from the end of the list. In my case I have a VecDeque of components that I use as a queue, so the IdList just doesn't really work in the first place.

2. Each item in the list consists of multiple components, and I have a function to render an item. The SoA style completely breaks encapsulation, as the calling code has to always know what components are going to be rendered. IMO this removes one of the key benefits of an immediate-mode GUI, which is the ability to have functions that can completely encapsulate all aspects of one part of the UI.

To me, the more annoying part is resizing the id::List. Due to how conrod is designed, you can't free the memory used by stale widgets and the IDs aren't automatically recycled

That does seem like a bit of an oversight. If a frame is rendered without a component, then that component should be considered deleted, and the memory associated with its ID freed. To prevent IDs being exhausted, 64-bit ID should be used.

[alvinhochun]

If you do this, it gets a bit inconvenient when you need to refer to the widget by its ID, e.g. when positioning and parenting other widgets.

I think this is only an issue if you need to refer to the widget before drawing it, which doesn't seem too much of a burden since you can't have cycles when positioning or parenting anyway. Also, this doesn't prohibit you from initializing specific IDs early if you need to refer to them before drawing the component.

This is somewhat true, but with your suggested API, one would still have to unwrap the Option after calling set_opt in order to get the ID. IMO doing let id = maybe_id.get_or_insert_with(|| ui_cell.widget_id_generator().next()) before setting the widget seems like a better approach.

TBH .zip()-ing the IDs with other states doesn't seem that annoying to me. I use .zip quite often in other code too when I have SoA (struct of array).

There are two problems with this beyond the ergonomics of zip.

1. The IdList only works if you only add and remove components to/from the end of the list. In my case I have a VecDeque of components that I use as a queue, so the IdList just doesn't really work in the first place.

You have a point. In cases like yours it seems that an Option<Id> should work fine, and it seems that's what you're already using.

2. Each item in the list consists of multiple components, and I have a function to render an item. The AoS style completely breaks encapsulation, as the calling code has to always know what components are going to be rendered.

In this case I think you can have a struct created with the widget_id! macro, then wrap it in an Option and use it with get_or_insert_with like above. Or depending on the use it might be better to make a custom Widget to combine the components into one.

In both of these cases I do agree that the ID list type provided by conrod doesn't really work well.

IMO this removes one of the key benefits of an immediate-mode GUI, which is the ability to have functions that can completely encapsulate all aspects of one part of the UI.

I think the inherent issue is the requirement of explicit IDs, as opposed to generating IDs automatically like Dear ImGui or others. But I think it works fine for me.

To me, the more annoying part is resizing the id::List. Due to how conrod is designed, you can't free the memory used by stale widgets and the IDs aren't automatically recycled

That does seem like a bit of an oversight. If a frame is rendered without a component, then that component should be considered deleted, and the memory associated with its ID freed. To prevent IDs being exhausted, 64-bit ID should be used.

I think this is debatable. Not setting a widget doesn't mean it'll not be used again, maybe the user just want it to be hidden for a while.

I don't think it is really realistic for an application using conrod to exceed 2^32 widget IDs, so u64 IDs would seem a bit overkill.

[Diggsey]

This is somewhat true, but with your suggested API, one would still have to unwrap the Option

I used an Option for my workaround, but if this was built into conrod I would expect the "nullability" aspect to be built into the ID type.

I think this is debatable. Not setting a widget doesn't mean it'll not be used again, maybe the user just want it to be hidden for a while.

I think that's fine as long as you don't expect the internal state of the widget to be preserved. If you want the state to be preserved when the widget is hidden, then "hidden" should be a property of the widget instead.

I don't think it is really realistic for an application using conrod to exceed 2^32 widget IDs, so u64 IDs would seem a bit overkill.

Certainly not at one time, but you could easily exceed 2^32 IDs over the lifetime of the program if widgets are dynamically created and deleted. Recycling the IDs themselves seems problematic unless you add a destructor to the Id type, but you should be able to recycle the memory referenced by those IDs.

edit:
This brings back memories, I made my own immediate-mode GUI a long time ago in a different language :)

[alvinhochun]

This is somewhat true, but with your suggested API, one would still have to unwrap the Option

I used an Option for my workaround, but if this was built into conrod I would expect the "nullability" aspect to be built into the ID type.

I don't think it is a good idea. Rust has Option for a reason.

[Diggsey]

I don't think it is a good idea. Rust has Option for a reason.

To ensure you handle the None case yeah. You can still get that benefit with an internally nullable ID: since "being not null" only matters for a select few operations (positioning or parenting) you can either:

1. Have those operations use a non-nullable ID type where conversion to that type will return an error for null IDs.
2. Have those operations return an error if a null ID is passed in.

Since the common case would be for IDs to be lazily initialized, it doesn't make sense for the caller to have to repeat Option<Id> everywhere. Having nullability be external makes sense when most operations would fail if the value is null (such as with a null reference).

[Diggsey]

TBH, you could even make the positioning/parenting operations initialize the ID if it is currently null, thereby making it completely transparent to the user.

[alvinhochun]

I don't think it is a good idea. Rust has Option for a reason.

To ensure you handle the None case yeah. You can still get that benefit with an internally nullable ID: since "being not null" only matters for a select few operations (positioning or parenting) you can either:

1. Have those operations use a non-nullable ID type where conversion to that type will return an error for null IDs.

2. Have those operations return an error if a null ID is passed in.

Since the common case would be for IDs to be lazily initialized, it doesn't make sense for the caller to have to repeat Option<Id> everywhere. Having nullability be external makes sense when most operations would fail if the value is null (such as with a null reference).

Nullable IDs still doesn't make much sense.

• Having separate non-null ID type and nullable ID type is pretty pointless given that one can already just use Option.
• Having the function return an error on a null ID:
  • Panicking would mean an extra chance of terminating the program unexpectedly. This is inferior to using a non-null ID type as the type system could have ensured that the null case would never happen.
  • Assuming you meant Result::Err, this would be an extra error case to handle, and very often it would just be pointless .expect() littered all over the place as the user anticipates it will never happen, which is the same as possibly panicking on null. There is also not much point in handling the error gracefully, because if the caller knows the ID may be null they could have just lazily initialize the ID before calling the function. With a non-null ID type and Option, using Option::get_or_insert_with would have the type system enforce that the ID must not be "null".

Also, using nullable IDs is a departure from idiomatic Rust. Passing Option<T> around when necessary is idiomatic Rust.

TBH, you could even make the positioning/parenting operations initialize the ID if it is currently null, thereby making it completely transparent to the user.

It would require taking &mut UiCell or id::Generator everywhere which will cause more inconvenience than anything else.

I think nullable IDs are the wrong thing to ask for. It probably makes more sense to consider implicitly-generated IDs like other immediate mode GUIs and make explicit ID optional. However, I don't know @mitchmindtree's reasoning on making IDs explicit in conrod, so I can't comment on this.

[Diggsey]

It probably makes more sense to consider implicitly-generated IDs like other immediate mode GUIs and make explicit ID optional. However, I don't know @mitchmindtree's reasoning on making IDs explicit in conrod, so I can't comment on this.

I don't know if you can make them completely implicit - you need some way to track component state across frames.

• You could do what React does and use a combination of component type/render order plus an optional local key value when components are rendered in a loop to match up the state.

• Or, you could have a global AtomicU64 counter so that unique IDs can be generated from anywhere, and you don't need special container types like IdList.

[alvinhochun]

Dear ImGui does it with an ID stack. The ID of a widget is typically generated from the label. Container widgets push its ID to the stack and the user can also push custom IDs to the stack to start a scope. The ID has to be popped at the end of the container or scope. A widget is identified by the hash of the widget "path", taken from the ID stack.

Ref: https://github.com/ocornut/imgui/blob/master/docs/FAQ.md#q-why-is-my-widget-not-reacting-when-i-click-on-it