How to integrate multi-frames icons/preview images in UI code #49820

New Issue

Bastien Montagne · 2016-10-22T22:39:43+02:00

Bastien Montagne commented

2016-10-22 22:39:43 +02:00

So, to support one preview per pose of a PoseLibrary (first use case, many more are possible later), multi-preview (or preview frames) is being added to Blender (see the branch for details).

The main issue I’m facing here is how to use those 'frames' of preview images. I can think of two solutions, neither are really ideal to my taste (went with first one in current branch):

Introduce the concept of 'frames' of an icon. You basically have to pass an extra value to UI API functions, the frame of the icon you want to use. This is simple and rather powerful, drawback here is that you'll ultimately have to change the whole UI API, which will be awfully… verbose.
Generate one icon ID per frame of the preview image. This is simpler, but can lead to a huge amount of generated icon id’s (with matching Icon data), imho it’s really not worth the overhead.

Wouldn’t mind having your feelings here, or even better if you have other ideas, before I change whole UI api…

So, to support one preview per pose of a PoseLibrary (first use case, many more are possible later), multi-preview (or preview frames) is being added to Blender (see [the branch](https://developer.blender.org/diffusion/B/browse/multi_previews_id/) for details). The main issue I’m facing here is how to use those 'frames' of preview images. I can think of two solutions, neither are really ideal to my taste (went with first one in current branch): - Introduce the concept of 'frames' of an icon. You basically have to pass an extra value to UI API functions, the frame of the icon you want to use. This is simple and rather powerful, drawback here is that you'll ultimately have to change the whole UI API, which will be awfully… verbose. - Generate one icon ID per frame of the preview image. This is simpler, but can lead to a huge amount of generated icon id’s (with matching Icon data), imho it’s really not worth the overhead. Wouldn’t mind having your feelings here, or even better if you have other ideas, before I change whole UI api…

Bastien Montagne commented

2016-10-22 22:39:43 +02:00

Changed status to: 'Open'

Bastien Montagne commented

2016-10-22 22:39:43 +02:00

Added subscribers: @mont29, @JulianEisel, @dr.sybren, @brecht

Brecht Van Lommel commented

2016-10-23 01:20:32 +02:00

I don't understand why generating one icon ID per frame would have significantly more overhead than multiple frames under one icon ID. It should be possible to have some data structure where memory usage is quite similar?

Bastien Montagne commented

2016-10-23 09:05:17 +02:00

The issue is that currently, you have one Icon struct per icon ID (stored in the global gIcons GHash). And we’d have to add to that struct a new short to store the frame of ImagePreview it refers to.

So in the perspective of pose library with tens or even hundreds of poses, and one preview frames for each pose, it might be adding a lot of those new Icon structs.

Also, (generated) Icon id system is currently bound to IDs, if we want to add it to sub-ID data, we'll have to add handling of icon_id for them…

So again, not sure which solution is best here, but I tend to dislike the idea of extending icon_id/Icon struct to PreviewImage frames, would rather keep the relation one icon_id <-> one ImagePreview, and add an optional extra parameter to index frame.

The issue is that currently, you have one Icon struct per icon ID (stored in the global gIcons GHash). And we’d have to add to that struct a new short to store the frame of ImagePreview it refers to. So in the perspective of pose library with tens or even hundreds of poses, and one preview frames for each pose, it might be adding a lot of those new Icon structs. Also, (generated) Icon id system is currently bound to IDs, if we want to add it to sub-ID data, we'll have to add handling of icon_id for them… So again, not sure which solution is best here, but I tend to dislike the idea of extending icon_id/Icon struct to PreviewImage frames, would rather keep the relation one icon_id <-> one ImagePreview, and add an optional extra parameter to index frame.

Sybren A. Stüvel commented

2016-10-23 09:39:06 +02:00

In #49820#400514, @mont29 wrote:
So in the perspective of pose library with tens or even hundreds of poses, and one preview frames for each pose, it might be adding a lot of those new Icon structs.

Every pose is defined by a pose marker. If we attach the preview of that pose to that marker, we still have a simple situation, right? Then we only have a constant number of preview frames per ID block.

We might want to have more than one preview frame per ID block, for example a large image and a thumbnail-sized image. However, that can be easily encoded into the least-significant bits of the generated ID.

> In #49820#400514, @mont29 wrote: > So in the perspective of pose library with tens or even hundreds of poses, and one preview frames for each pose, it might be adding a lot of those new Icon structs. Every pose is defined by a pose marker. If we attach the preview of that pose to that marker, we still have a simple situation, right? Then we only have a constant number of preview frames per ID block. We might want to have more than one preview frame per ID block, for example a large image and a thumbnail-sized image. However, that can be easily encoded into the least-significant bits of the generated ID.

Bastien Montagne commented

2016-10-23 09:55:09 +02:00

@dr.sybren no, same as with icon_id, I would not spread PreviewImage into sub-ID data, this will quickly become unmanageable.

The whole idea of the branch is to be able to add more than one preview image to ID previews, such that sub-data can then use those additional images as their own preview (and maybe later, to have animated previews of actions or videos e.g.…).

And we already have an icon vs. 'full size' preview system, no need for anything new here. But this has nothing to do with 'preview frames'.

@dr.sybren no, same as with icon_id, I would not spread PreviewImage into sub-ID data, this will quickly become unmanageable. The whole idea of the branch is to be able to add more than one preview image to ID previews, such that sub-data can then use those additional images as their own preview (and maybe later, to have animated previews of actions or videos e.g.…). And we already have an icon vs. 'full size' preview system, no need for anything new here. But this has nothing to do with 'preview frames'.

Brecht Van Lommel commented

2016-10-23 17:25:48 +02:00

If the ideas is to have animations at some point, then such frames should be a separate concept from sub-ID icons I think? Animation playback would be handled by the UI code I guess and not through Python.

In #49820#400514, @mont29 wrote:
The issue is that currently, you have one Icon struct per icon ID (stored in the global gIcons GHash). And we’d have to add to that struct a new short to store the frame of ImagePreview it refers to.

So in the perspective of pose library with tens or even hundreds of poses, and one preview frames for each pose, it might be adding a lot of those new Icon structs.

Is Icon struct memory usage significant compared to the pixel memory usage? If it is a concern, its memory usage can be reduced already by replacing drawinfo_free with a global callback variable, and sharing the same pointer for drawinfo and obj, but it all seems to be quite minor in comparison to the whole.

Also, (generated) Icon id system is currently bound to IDs, if we want to add it to sub-ID data, we'll have to add handling of icon_id for them…

Not sure I understand, one way or the other you need to have some way get a frame number corresponding to some sub-ID data? So might as well return a single icon ID value then?

If the ideas is to have animations at some point, then such frames should be a separate concept from sub-ID icons I think? Animation playback would be handled by the UI code I guess and not through Python. > In #49820#400514, @mont29 wrote: > The issue is that currently, you have one Icon struct per icon ID (stored in the global gIcons GHash). And we’d have to add to that struct a new short to store the frame of ImagePreview it refers to. > > So in the perspective of pose library with tens or even hundreds of poses, and one preview frames for each pose, it might be adding a lot of those new Icon structs. Is `Icon` struct memory usage significant compared to the pixel memory usage? If it is a concern, its memory usage can be reduced already by replacing `drawinfo_free` with a global callback variable, and sharing the same pointer for `drawinfo` and `obj`, but it all seems to be quite minor in comparison to the whole. > Also, (generated) Icon id system is currently bound to IDs, if we want to add it to sub-ID data, we'll have to add handling of icon_id for them… Not sure I understand, one way or the other you need to have some way get a frame number corresponding to some sub-ID data? So might as well return a single icon ID value then?

Bastien Montagne commented

2016-10-23 17:40:57 +02:00

In #49820#400562, @brecht wrote:
If the ideas is to have animations at some point, then such frames should be a separate concept from sub-ID icons I think? Animation playback would be handled by the UI code I guess and not through Python.

All are handled by UI code in the end anyway, idea of current design is to allow both with the same backend storage (set of contiguous frames in ImagePreview struct). But that’s slightly out of the scope anyway, animated preview is just idea for future so far.

Is Icon struct memory usage significant compared to the pixel memory usage? If it is a concern, its memory usage can be reduced already by replacing drawinfo_free with a global callback variable, and sharing the same pointer for drawinfo and obj, but it all seems to be quite minor in comparison to the whole.

No, size is not really an issue. It would add some overhead (something like 24-32 bytes per frame), but this is not critical imho.

Not sure I understand, one way or the other you need to have some way get a frame number corresponding to some sub-ID data? So might as well return a single icon ID value then?

The issue here is that dynamic icon ID system is designed to work with IDs, if we want to use it elsewhere we'll have to extend its API. Further more, icon_id is fully runtime data, generated on demand, so we'll still need to store the frame index anyway. Using icon_id here would only mean we add an extra step to generate it (and again, it would break the "one icon_id <-> one PreviewImage" relationship we have currently).

> In #49820#400562, @brecht wrote: > If the ideas is to have animations at some point, then such frames should be a separate concept from sub-ID icons I think? Animation playback would be handled by the UI code I guess and not through Python. All are handled by UI code in the end anyway, idea of current design is to allow both with the same backend storage (set of contiguous frames in ImagePreview struct). But that’s slightly out of the scope anyway, animated preview is just idea for future so far. > Is `Icon` struct memory usage significant compared to the pixel memory usage? If it is a concern, its memory usage can be reduced already by replacing `drawinfo_free` with a global callback variable, and sharing the same pointer for `drawinfo` and `obj`, but it all seems to be quite minor in comparison to the whole. No, size is not really an issue. It would add some overhead (something like 24-32 bytes per frame), but this is not critical imho. > Not sure I understand, one way or the other you need to have some way get a frame number corresponding to some sub-ID data? So might as well return a single icon ID value then? The issue here is that dynamic icon ID system is designed to work with IDs, if we want to use it elsewhere we'll have to extend its API. Further more, icon_id is fully runtime data, generated on demand, so we'll still need to store the frame index anyway. Using icon_id here would only mean we add an extra step to generate it (and again, it would break the "one icon_id <-> one PreviewImage" relationship we have currently).

Joshua Leung commented

2016-10-24 05:45:27 +02:00

Added subscriber: @JoshuaLeung

Joshua Leung commented

2016-10-24 05:45:27 +02:00

Are there any docs on how this whole PreviewImage system currently works?

I have to agree with Sybren here that the obvious approach to solve the problem we originally had here (i.e. the need to be able to store preview images for each pose in a pose library) would have been to stick those preview images on each marker. Or, if that's too much overhead, to stick the collection of them in each action. Back when I was thinking of looking into this sometime in the future, that was what I was going to do.

If we later wanted to extend this type of functionality so that you could have say a short 5-10 second "preview" of an action (for use in the asset browser), then that case could be handled by doing something like:

Static Icon (i.e. the existing, single preview image you get per ID now)
Preview Frames - A set of additional sub-ID images that can be accessed and played back
Your existing API's can just be kept as is. They will only ever access the Static Icon. Alongside this, you add a second parallel API that supports these new multi-frame icon id's (e.g. you'd have your main icon_id, plus a secondary parameter which specifies which preview frame to take).

As for how this all functions on the backend, here's an idea:

As you're doing now, there's still the Global GHash that does a mapping from each icon_id to the corresponding single-static-icon. This is basically everything that happens now.
To support preview frames (or really, sub-id icons), you'd change things so that if an ID can have sub-id icons, its icon_id never gets added to this Global GHash.
1. Instead, you could have a GHash on each relevant ID, that will store that all of that ID's icons (static + preview frames). Then, you'd change the lookup magic so that if if doesn't find something in the global ghash, it hunts for stuff in the ID.
2. Or, if you really do need a way to figure out which datablock to fetch from, you could have a secondary GHash for these cases, and make it so that the ID's with subframes get stored there. The keys would be the ID-level icon_id's, and the values pointers to the per-ID sub_id_icons_ghash. If your lookup fails to find an icon_id in the first collection, it goes to the second. If it finds a match in the second, it goes inside that to find the sub-icon required.

Also, is it really such a big deal that we have to "extend" some API's? I mean, there are numerous examples in the codebase now where we've got the "old/simple" version that was kept for all the existing places where that API is used, and a *_ex() version that takes additional parameters supporting the new stuff. I'm sure that not all of the stuff in the API really needs to be extended like this...

(Besides, it's not like we really have a publically exposed C-API with all this stuff that we need to worry about! The bpy integration can adapt really easily to this sort of stuff - i.e. just make your new parameters optional, and the old code won't break; for extra brownie points, pre-empt this change in older versions by adding "extra parameter catch-bag dicts" to the API's so that the extensions you add in a future release don't break API's that easily.)

Are there any docs on how this whole PreviewImage system currently works? I have to agree with Sybren here that the obvious approach to solve the problem we originally had here (i.e. the need to be able to store preview images for each pose in a pose library) would have been to stick those preview images on each marker. Or, if that's too much overhead, to stick the collection of them in each action. Back when I was thinking of looking into this sometime in the future, that was what I was going to do. If we later wanted to extend this type of functionality so that you could have say a short 5-10 second "preview" of an action (for use in the asset browser), then that case could be handled by doing something like: * Static Icon (i.e. the existing, single preview image you get per ID now) * Preview Frames - A set of additional sub-ID images that can be accessed and played back * Your existing API's can just be kept as is. They will only ever access the Static Icon. Alongside this, you add a second parallel API that supports these new multi-frame icon id's (e.g. you'd have your main icon_id, plus a secondary parameter which specifies which preview frame to take). As for how this all functions on the backend, here's an idea: * As you're doing now, there's still the Global GHash that does a mapping from each icon_id to the corresponding single-static-icon. This is basically everything that happens now. * To support preview frames (or really, sub-id icons), you'd change things so that if an ID can have sub-id icons, its icon_id never gets added to this Global GHash. 1) Instead, you could have a GHash on each relevant ID, that will store that all of that ID's icons (static + preview frames). Then, you'd change the lookup magic so that if if doesn't find something in the global ghash, it hunts for stuff in the ID. 2) Or, if you really do need a way to figure out which datablock to fetch from, you could have a secondary GHash for these cases, and make it so that the ID's with subframes get stored there. The keys would be the ID-level icon_id's, and the values pointers to the per-ID sub_id_icons_ghash. If your lookup fails to find an icon_id in the first collection, it goes to the second. If it finds a match in the second, it goes inside that to find the sub-icon required. Also, is it **really** such a big deal that we have to "extend" some API's? I mean, there are numerous examples in the codebase now where we've got the "old/simple" version that was kept for all the existing places where that API is used, and a `*_ex()` version that takes additional parameters supporting the new stuff. I'm sure that not *all* of the stuff in the API really needs to be extended like this... (Besides, it's not like we really have a publically exposed C-API with all this stuff that we need to worry about! The bpy integration can adapt really easily to this sort of stuff - i.e. just make your new parameters optional, and the old code won't break; for extra brownie points, pre-empt this change in older versions by adding "extra parameter catch-bag dicts" to the API's so that the extensions you add in a future release don't break API's that easily.)

Philipp Oeser commented

2016-10-24 09:39:54 +02:00

Added subscriber: @lichtwerk

Bastien Montagne commented

2016-10-24 10:00:22 +02:00

In #49820#400626, @JoshuaLeung wrote:
Are there any docs on how this whole PreviewImage system currently works?

No, as often, only code :P

I have to agree with Sybren here that the obvious approach to solve the problem we originally had here (i.e. the need to be able to store preview images for each pose in a pose library) would have been to stick those preview images on each marker. Or, if that's too much overhead, to stick the collection of them in each action. Back when I was thinking of looking into this sometime in the future, that was what I was going to do.

Actually, new code in branch is exactly doing that second suggestion, but in an even more 'compact' way, since it makes PreviewImage able to store mutiple “frames”, in a way that keeps it fully compatible with its previous single framed version. So the PreviewImage of Action datablock will store all images, but keeps same behavior by default (i.e. if you access its content th old way, you just get first frame).

If we later wanted to extend this type of functionality so that you could have say a short 5-10 second "preview" of an action (for use in the asset browser), then that case could be handled by doing something like:

Static Icon (i.e. the existing, single preview image you get per ID now)

Preview Frames - A set of additional sub-ID images that can be accessed and played back

Your existing API's can just be kept as is. They will only ever access the Static Icon. Alongside this, you add a second parallel API that supports these new multi-frame icon id's (e.g. you'd have your main icon_id, plus a secondary parameter which specifies which preview frame to take).

Animation case is actually simpler imho, you can just add a new flag to UI code to say to buttons 'this icon_id is animated' (and then UI code internally uses that info smartly - probably e.g. only playing animation when button is in active sate, etc.).

As for how this all functions on the backend, here's an idea:

As you're doing now, there's still the Global GHash that does a mapping from each icon_id to the corresponding single-static-icon. This is basically everything that happens now.

To support preview frames (or really, sub-id icons), you'd change things so that if an ID can have sub-id icons, its icon_id never gets added to this Global GHash.

Instead, you could have a GHash on each relevant ID, that will store that all of that ID's icons (static + preview frames). Then, you'd change the lookup magic so that if if doesn't find something in the global ghash, it hunts for stuff in the ID.

Or, if you really do need a way to figure out which datablock to fetch from, you could have a secondary GHash for these cases, and make it so that the ID's with subframes get stored there. The keys would be the ID-level icon_id's, and the values pointers to the per-ID sub_id_icons_ghash. If your lookup fails to find an icon_id in the first collection, it goes to the second. If it finds a match in the second, it goes inside that to find the sub-icon required.

Hmmm… this sounds horribly complicated to me? I mean, if we want to really just use icon_id also for 'frames', then we just generate more Icon, and add a new 'frame' short to Icon struct, and we are done? It’s not that complicated, am just not so happy to extend the icon_id concept to sub_ID data.

Also, is it really such a big deal that we have to "extend" some API's? I mean, there are numerous examples in the codebase now where we've got the "old/simple" version that was kept for all the existing places where that API is used, and a *_ex() version that takes additional parameters supporting the new stuff. I'm sure that not all of the stuff in the API really needs to be extended like this...

(Besides, it's not like we really have a publically exposed C-API with all this stuff that we need to worry about! The bpy integration can adapt really easily to this sort of stuff - i.e. just make your new parameters optional, and the old code won't break; for extra brownie points, pre-empt this change in older versions by adding "extra parameter catch-bag dicts" to the API's so that the extensions you add in a future release don't break API's that easily.)

Yes, I would really rather like to add extra 'frame index' parameter to our UI api (that’s what I partially did in branch already). In theory we should not have to add an extra parameter to C API, for uiBut functions e.g., since they return a button pointer, we can just add an extra helper function to set frame of icon to use in the few cases where we need other than first one, and that’s it. The issue is with the uiLayout functions - those do not return any handle of any kind, so we cannot use that 'extra optional step' approach, and have to update the whole set of functions to accept an extra parameter.

Nothing deadly here, but this is going to be noisy, and potentially annoying for other UI work done in branches…

> In #49820#400626, @JoshuaLeung wrote: > Are there any docs on how this whole PreviewImage system currently works? No, as often, only code :P > I have to agree with Sybren here that the obvious approach to solve the problem we originally had here (i.e. the need to be able to store preview images for each pose in a pose library) would have been to stick those preview images on each marker. Or, if that's too much overhead, to stick the collection of them in each action. Back when I was thinking of looking into this sometime in the future, that was what I was going to do. Actually, new code in branch is exactly doing that second suggestion, but in an even more 'compact' way, since it makes PreviewImage able to store mutiple “frames”, in a way that keeps it fully compatible with its previous single framed version. So the PreviewImage of Action datablock will store all images, but keeps same behavior by default (i.e. if you access its content th old way, you just get first frame). > If we later wanted to extend this type of functionality so that you could have say a short 5-10 second "preview" of an action (for use in the asset browser), then that case could be handled by doing something like: > * Static Icon (i.e. the existing, single preview image you get per ID now) > * Preview Frames - A set of additional sub-ID images that can be accessed and played back > * Your existing API's can just be kept as is. They will only ever access the Static Icon. Alongside this, you add a second parallel API that supports these new multi-frame icon id's (e.g. you'd have your main icon_id, plus a secondary parameter which specifies which preview frame to take). Animation case is actually simpler imho, you can just add a new flag to UI code to say to buttons 'this icon_id is animated' (and then UI code internally uses that info smartly - probably e.g. only playing animation when button is in active sate, etc.). > As for how this all functions on the backend, here's an idea: > * As you're doing now, there's still the Global GHash that does a mapping from each icon_id to the corresponding single-static-icon. This is basically everything that happens now. > * To support preview frames (or really, sub-id icons), you'd change things so that if an ID can have sub-id icons, its icon_id never gets added to this Global GHash. > 1) Instead, you could have a GHash on each relevant ID, that will store that all of that ID's icons (static + preview frames). Then, you'd change the lookup magic so that if if doesn't find something in the global ghash, it hunts for stuff in the ID. > > 2) Or, if you really do need a way to figure out which datablock to fetch from, you could have a secondary GHash for these cases, and make it so that the ID's with subframes get stored there. The keys would be the ID-level icon_id's, and the values pointers to the per-ID sub_id_icons_ghash. If your lookup fails to find an icon_id in the first collection, it goes to the second. If it finds a match in the second, it goes inside that to find the sub-icon required. Hmmm… this sounds horribly complicated to me? I mean, if we want to really just use icon_id also for 'frames', then we just generate more Icon, and add a new 'frame' short to Icon struct, and we are done? It’s not that complicated, am just not so happy to extend the icon_id concept to sub_ID data. > Also, is it **really** such a big deal that we have to "extend" some API's? I mean, there are numerous examples in the codebase now where we've got the "old/simple" version that was kept for all the existing places where that API is used, and a `*_ex()` version that takes additional parameters supporting the new stuff. I'm sure that not *all* of the stuff in the API really needs to be extended like this... > > (Besides, it's not like we really have a publically exposed C-API with all this stuff that we need to worry about! The bpy integration can adapt really easily to this sort of stuff - i.e. just make your new parameters optional, and the old code won't break; for extra brownie points, pre-empt this change in older versions by adding "extra parameter catch-bag dicts" to the API's so that the extensions you add in a future release don't break API's that easily.) Yes, I would really rather like to add extra 'frame index' parameter to our UI api (that’s what I partially did in branch already). In theory we should not have to add an extra parameter to C API, for uiBut functions e.g., since they return a button pointer, we can just add [an extra helper function](https://developer.blender.org/diffusion/B/browse/multi_previews_id/source/blender/editors/interface/interface_intern.h;fc383cd56ef9223ef2087001a810f56c5002886e$486) to set frame of icon to use in the few cases where we need other than first one, and that’s it. The issue is with the uiLayout functions - those do not return any handle of any kind, so we cannot use that 'extra optional step' approach, and have to update the whole set of functions to accept an extra parameter. Nothing deadly here, but this is going to be noisy, and potentially annoying for other UI work done in branches…

Philipp Oeser removed the

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

How to integrate multi-frames icons/preview images in UI code #49820