VSE: Make cache overlay visible to non "Developer extras" users #119428

Merged
Sebastian Parborg merged 2 commits from ZedDB/blender:vse_cache_overlay into main 2024-05-02 16:36:21 +02:00

This also moves the option to be a per editor space setting so you can
have different cache visualization options in different editor spaces
at the same time.

A quick toggle for the cache visualization is now available in the
overlays popover.

To follow the cache settings in the timeline, I enabled all cache options. But perhaps we want to only show the "final image" cache per default? Note that is easy to toggle this on/off now in the overlay settings, so perhaps having everything on is ok? The additional cache options shouldn't occlude anything as thumbnails are off per default.

This also moves the option to be a per editor space setting so you can have different cache visualization options in different editor spaces at the same time. A quick toggle for the cache visualization is now available in the overlays popover. To follow the cache settings in the timeline, I enabled all cache options. But perhaps we want to only show the "final image" cache per default? Note that is easy to toggle this on/off now in the overlay settings, so perhaps having everything on is ok? The additional cache options shouldn't occlude anything as thumbnails are off per default.
Sebastian Parborg force-pushed vse_cache_overlay from 194f453a5a to 6ed860979c 2024-03-13 17:08:47 +01:00 Compare
Sebastian Parborg requested review from Sergey Sharybin 2024-03-13 17:11:41 +01:00
Sebastian Parborg requested review from Richard Antalik 2024-03-13 17:11:41 +01:00
Sebastian Parborg added the
Module
VFX & Video
label 2024-03-13 17:11:55 +01:00

I think, that the idea was to only show final images without developer extras. For normal users it would not be of any benefit to see other types I think.

I think, that the idea was to only show final images without developer extras. For normal users it would not be of any benefit to see other types I think.
Richard Antalik reviewed 2024-03-13 21:16:36 +01:00
Richard Antalik left a comment
Member

From code perspective, seems OK. I would probably move cache tab from sidebar to overlays panel, so you have all stuff together. If this is hidden by developer extras it could be 3 joined buttons for "cache extras".

From code perspective, seems OK. I would probably move cache tab from sidebar to overlays panel, so you have all stuff together. If this is hidden by developer extras it could be 3 joined buttons for "cache extras".
@ -812,3 +809,1 @@
SEQ_CACHE_VIEW_PREPROCESSED = (1 << 7),
SEQ_CACHE_VIEW_COMPOSITE = (1 << 8),
SEQ_CACHE_VIEW_FINAL_OUT = (1 << 9),
/* Deprecated: CACHE_VIEW setting has been moved to CACHE_SHOW in DNA_space_types.h

I would just remove these definitions, especially if there is no versioning.

I would just remove these definitions, especially if there is no versioning.
Author
Member

I left them in here in case someone tries to reuse them. If I just delete them, I think it might be easy to forget to clear these in the versioning code.

Or perhaps I should just delete these and add a clear entry in versioning right now?

I left them in here in case someone tries to reuse them. If I just delete them, I think it might be easy to forget to clear these in the versioning code. Or perhaps I should just delete these and add a clear entry in versioning right now?
ZedDB marked this conversation as resolved
@ -646,6 +646,20 @@ typedef enum eSpaceSeq_SequencerTimelineOverlay_Flag {
SEQ_TIMELINE_SHOW_GRID = (1 << 18),
} eSpaceSeq_SequencerTimelineOverlay_Flag;
typedef struct SequencerCacheOverlay {

Not sure if it is necessary to make new struct for this. Functionally and semantically it would be just as fine under SequencerTimelineOverlay.

Not sure if it is necessary to make new struct for this. Functionally and semantically it would be just as fine under `SequencerTimelineOverlay`.
Author
Member

I made a new entry as the SEQ_TIMELINE_ flags would only have 8 entries left before it would be full. I think this might also make it more organized if we for what ever reason need to add more cache options in to future.

Do you have any strong opinions about this?

I made a new entry as the `SEQ_TIMELINE_` flags would only have 8 entries left before it would be full. I think this might also make it more organized if we for what ever reason need to add more cache options in to future. Do you have any strong opinions about this?

Not really a strong opinion. But bits 10-13 are free, SEQ_CACHE_SHOW_FINAL_OUT could be implied by SEQ_CACHE_SHOW. so this is squeezable :) I would do it...

Not really a strong opinion. But bits 10-13 are free, `SEQ_CACHE_SHOW_FINAL_OUT` could be implied by `SEQ_CACHE_SHOW`. so this is squeezable :) I would do it...
Author
Member

Ehh... I don't really like to try to squeeze things into other places if it is easy to just do a clean struct that has a clear category and intent. Especially because this area might change a bit in the future.

Ehh... I don't really like to try to squeeze things into other places if it is easy to just do a clean struct that has a clear category and intent. Especially because this area might change a bit in the future.

It is possible to share this cache by multiple copies of object before first change?

It is possible to share this cache by multiple copies of object before first change?
ZedDB marked this conversation as resolved
Sergey Sharybin requested changes 2024-03-14 10:01:23 +01:00
Dismissed
Sergey Sharybin left a comment
Owner

The description could use a motivational part about why artists need to worry about something that hd a clear intent of a developer-only functionality.

As for the level of caches, I do not really see why artists will worry about anything but the final images. So at a very least, the other caches are to be disabled by default.
But I'd also go as far as saying that for the artists without Developer Extras there should be a single "Show Cache" option. The granularity of levels could then be done and shown when Developers Extras option is enabled.

The description could use a motivational part about why artists need to worry about something that hd a clear intent of a developer-only functionality. As for the level of caches, I do not really see why artists will worry about anything but the final images. So at a very least, the other caches are to be disabled by default. But I'd also go as far as saying that for the artists without Developer Extras there should be a single "Show Cache" option. The granularity of levels could then be done and shown when Developers Extras option is enabled.
@ -341,3 +342,3 @@
col = layout.column()
col.enabled = ed.show_cache
col.enabled = cache_settings.show_cache

Use active instead of enabled. The enabled is only to be used in cases when changing property is not possible at all, without causing some serious side effects (like, crashes). If the property contextually is not used, then artists should still be able to modify the value.

It similarly, say, how you can tweak simplification settings in the Simply panel without actually enabling simplification.

Use `active` instead of `enabled`. The `enabled` is only to be used in cases when changing property is not possible at all, without causing some serious side effects (like, crashes). If the property contextually is not used, then artists should still be able to modify the value. It similarly, say, how you can tweak simplification settings in the Simply panel without actually enabling simplification.
ZedDB marked this conversation as resolved

I was giving more testing to the patch, and now I am definitely convinced with the current state of the cache visualization anything but the final-image cache is definitely a no-go.

Looking at the current edit of the project: Screenshot 2024-03-14 at 11.32.04.png
That per-strip visualization is going to interfere with thumbnails, possibly waveforms, missing media visualization, retiming, and possibly other things which do not pop up in mind immediatelly.

Additionally, the visualuzation of cached for metastrips seems to be broken: Screenshot 2024-03-14 at 11.35.24.png

I was giving more testing to the patch, and now I am definitely convinced with the current state of the cache visualization anything but the final-image cache is definitely a no-go. Looking at the current edit of the project: ![Screenshot 2024-03-14 at 11.32.04.png](/attachments/afc6988f-ee1e-4ff7-8e26-0a2f48dadbe9) That per-strip visualization is going to interfere with thumbnails, possibly waveforms, missing media visualization, retiming, and possibly other things which do not pop up in mind immediatelly. Additionally, the visualuzation of cached for metastrips seems to be broken: ![Screenshot 2024-03-14 at 11.35.24.png](/attachments/e024edcc-1152-48f8-b615-10a63e51a76f)
Sergey Sharybin requested changes 2024-03-14 11:41:12 +01:00
Dismissed
Sergey Sharybin left a comment
Owner

Marking as requested changes.

Marking as requested changes.
Author
Member

I also noticed yesterday after posting this PR that at least the other cache options are at least broken visually. For example if you enable the cache for composite and preprossessed images, it seems to work when you first fill up the cache.

However if you then move the strips, the cache lines doesn't move to the correct frame.
I looked at the code and the reason for this is that the cache stores the absolute timecode for the cache and uses this to visualize where to draw the cache lines.

This works fine for final image caches, but not for per strip caches.

image
image

The code does store the actual relative frame of the strip as well, I think this might be an easy fix. I'll add a commit with the fix here soonish.

I do agree that we probably only want to have the final image cache option on per default.

But I'd also go as far as saying that for the artists without Developer Extras there should be a single "Show Cache" option. The granularity of levels could then be done and shown when Developers Extras option is enabled.

I don't think hiding the extra options behind developers extras is a good idea. Especially because users can the get files with the options turned on forcing them to either go through python or enabling the developers extras to turn it off.

The options are already hidden away enough as it is I feel.

The reason why we are moving this out of developers extras is that end users would be very confused why their playback performance were so inconsistent. They didn't think Blender had any caching as we hid away the options for it. So instead they just thought Blender was very crappy as it would playback sections fine sometimes and sometimes it would stutter like crazy.

With the ability to see the cache, they can now tell that the stuttering is because the cache is not populated.

I also noticed yesterday after posting this PR that at least the other cache options are at least broken visually. For example if you enable the cache for composite and preprossessed images, it seems to work when you first fill up the cache. However if you then move the strips, the cache lines doesn't move to the correct frame. I looked at the code and the reason for this is that the cache stores the absolute timecode for the cache and uses this to visualize where to draw the cache lines. This works fine for final image caches, but not for per strip caches. ![image](/attachments/3bf9418f-d447-44b2-8236-57487cbd3431) ![image](/attachments/678b2224-6d61-4e74-baaf-06ad443fb26f) The code does store the actual relative frame of the strip as well, I think this might be an easy fix. I'll add a commit with the fix here soonish. I do agree that we probably only want to have the final image cache option on per default. > But I'd also go as far as saying that for the artists without Developer Extras there should be a single "Show Cache" option. The granularity of levels could then be done and shown when Developers Extras option is enabled. I don't think hiding the extra options behind developers extras is a good idea. Especially because users can the get files with the options turned on forcing them to either go through python or enabling the developers extras to turn it off. The options are already hidden away enough as it is I feel. The reason why we are moving this out of developers extras is that end users would be very confused why their playback performance were so inconsistent. They didn't think Blender had any caching as we hid away the options for it. So instead they just thought Blender was very crappy as it would playback sections fine sometimes and sometimes it would stutter like crazy. With the ability to see the cache, they can now tell that the stuttering is because the cache is not populated.

Look into how, for example, Debug panels work in Cycles. There are a lot of options exposed there which could totally change certain algorithms, and they are saved and loaded into .blend file, but only have effect if the Developers Exrta + Cycles Debug is enabled. All the low-level debugging functionality in Sequencer should behave the same.

Having cache exposed at all is already quite a stretch, and is only acceptable because there is no time from a capable developer to solve the playback issues. Having a single cache display has a very simple mental model: frame cached == fast, frame non-cached == hiccups. I do not see any reason why regular (even highly experienced) video editors would need to worry about all those lower-level and technical details of extra levels of caching.

Saying that option is hidden enough is not a good excuse for compromising quality and clarity of the UI/UX.

Look into how, for example, Debug panels work in Cycles. There are a lot of options exposed there which could totally change certain algorithms, and they are saved and loaded into .blend file, but only have effect if the Developers Exrta + Cycles Debug is enabled. All the low-level debugging functionality in Sequencer should behave the same. Having cache exposed at all is already quite a stretch, and is only acceptable because there is no time from a capable developer to solve the playback issues. Having a single cache display has a very simple mental model: frame cached == fast, frame non-cached == hiccups. I do not see any reason why regular (even highly experienced) video editors would need to worry about all those lower-level and technical details of extra levels of caching. Saying that option is hidden enough is not a good excuse for compromising quality and clarity of the UI/UX.
Author
Member

I've disabled the drawing of the strip (non final) caches unless developer extras is on. I also made a quick fix to improve the drawing of the strip caches. They now move with the strips correctly. However they are still broken for meta strips and video strips with re-timing etc. As we only show these for developers, I opted not to work on that part any further as it would require quite a bit of extra code.

I'm waiting for Richard's replies before I act on his feedback.

I've disabled the drawing of the strip (non final) caches unless developer extras is on. I also made a quick fix to improve the drawing of the strip caches. They now move with the strips correctly. However they are still broken for meta strips and video strips with re-timing etc. As we only show these for developers, I opted not to work on that part any further as it would require quite a bit of extra code. I'm waiting for Richard's replies before I act on his feedback.

When a video editing project is created from scratch the option behaves very close to what I'd expect it to. At least on the menu/option level. It also gave a chance to play around in more details, and see how it feels and behaves.
The drawing itself could become but more about it later.

Opening an existing file and enabling cache display does not actually display cache. I guess it is because some code only initializes individual caches for new files, and drawing code relies on it, even when developer UI option is disabled.

Now, back to the UI and drawing.

The simple one: the option in Overlay menu needs to be called Cache (not plural).

The readability of the cache line could become very bad very quickly. Check the scheenshot: Screenshot 2024-03-18 at 15.09.49.png

The quick solution I can think of is to move the cache line to the top, and make it non-overlap drawing. Also, have it fixed size in UI pixels
(reglardless of the zoom level).
There is also an issue when horizontal zoom is small: it might be hard to see a gap. This needs a separate solution, but it is not something that people brought up with a cache line in Clip or Image Editor, so perhaps is not that important.

When a video editing project is created from scratch the option behaves very close to what I'd expect it to. At least on the menu/option level. It also gave a chance to play around in more details, and see how it feels and behaves. The drawing itself could become but more about it later. Opening an existing file and enabling cache display does not actually display cache. I guess it is because some code only initializes individual caches for new files, and drawing code relies on it, even when developer UI option is disabled. Now, back to the UI and drawing. The simple one: the option in Overlay menu needs to be called `Cache` (not plural). The readability of the cache line could become very bad very quickly. Check the scheenshot: ![Screenshot 2024-03-18 at 15.09.49.png](/attachments/de361967-db26-4f9a-a10e-b6b6c8b54563) The quick solution I can think of is to move the cache line to the top, and make it non-overlap drawing. Also, have it fixed size in UI pixels (reglardless of the zoom level). There is also an issue when horizontal zoom is small: it might be hard to see a gap. This needs a separate solution, but it is not something that people brought up with a cache line in Clip or Image Editor, so perhaps is not that important.
Sebastian Parborg force-pushed vse_cache_overlay from d0957c8a2b to 3c87ecbbde 2024-04-11 15:23:34 +02:00 Compare
Author
Member

I moved the cache drawing to be on top of the timeline:
image

Still waiting for Pablo to provide feedback if he has any

I moved the cache drawing to be on top of the timeline: ![image](/attachments/de02d5a8-54e3-4c28-9d86-1259b5570fe2) Still waiting for Pablo to provide feedback if he has any

I would try to make height a bit thinner, since it is quite prominent in such thin region, but not sure if it would be better. Perhaps different color could be used to make this less prominent, but still visible.

I would try to make height a bit thinner, since it is quite prominent in such thin region, but not sure if it would be better. Perhaps different color could be used to make this less prominent, but still visible.

@fsiddi @pablovazquez Are you checking on this from the design perspective?

@fsiddi @pablovazquez Are you checking on this from the design perspective?

This is 1/10 of scrubbing region height. I like it a bit more, since current frame indicator does not intersect it

Screenshot_2024-04-16_21-55-33.png

Tried gray color as well, but I don't think this is great:
Screenshot_2024-04-16_22-01-30.png

On that note, since this is visible by default now, it would be good idea to have a theme for these colors.

This is 1/10 of scrubbing region height. I like it a bit more, since current frame indicator does not intersect it ![Screenshot_2024-04-16_21-55-33.png](/attachments/b20924d7-1184-4294-aa6a-e885c154f29b) Tried gray color as well, but I don't think this is great: ![Screenshot_2024-04-16_22-01-30.png](/attachments/1f2079be-dd83-49d9-909a-c7f7a65badbd) On that note, since this is visible by default now, it would be good idea to have a theme for these colors.
Member

it would be good idea to have a theme for these colors.

No need for even more theme options. For over a decade we have the concept of cached media in the Movie Editor and Clip Editor.
Image Editor
We can simply re-use that. These colors have always been hardcoded and it hasn't really been a problem. If it becomes a problem one day we could add a global setting.


The cache line should not overlap. It should add padding to the top of the time scrubbing region, pushing all content down slightly. The following mockup uses 4px (twice the width of the current frame indicator line).

Cache line disabled Cache line enabled
current 4px
> it would be good idea to have a theme for these colors. No need for even more theme options. For over a decade we have the concept of cached media in the Movie Editor and Clip Editor. ![Image Editor](/attachments/5438917f-38b0-454b-b0fc-e469b5999e81) We can simply re-use that. These colors have always been hardcoded and it hasn't really been a problem. If it becomes a problem one day we could add a global setting. ---- The cache line should not overlap. It should add padding to the top of the time scrubbing region, pushing all content down slightly. The following mockup uses 4px (twice the width of the current frame indicator line). |Cache line disabled|Cache line enabled| |----|----| |![current](/attachments/72ab8b56-714a-4226-bd93-840243d86b1f)|![4px](/attachments/86485328-2a80-4a90-9d4d-f70c79f1f304)|
224 KiB
122 KiB
225 KiB
Member

For the record, we discussed this in person and agreed on trying out the cache lines under the time-scrub area instead (mainly for convenience and future proof with more cache types).

For the record, we discussed this in person and agreed on trying out the cache lines under the time-scrub area instead (mainly for convenience and future proof with more cache types).
Author
Member

I did a version last week with the cache line under the scrub area:
image

It does look ok when zoomed in. But when I zoom out, I feel like it kinda blends into the the rest of the UI:

image

Of course this is probably a contrived case, but I just wanted to exaggerate the effect to make my point more clear.

Probably nothing that we could fix with some themeing, but I wanted to check in what other are feeling.

I did a version last week with the cache line under the scrub area: ![image](/attachments/c982ddf1-001d-46da-8ee1-dc0aeac6ccb6) It does look ok when zoomed in. But when I zoom out, I feel like it kinda blends into the the rest of the UI: ![image](/attachments/a826be88-3ac7-4778-87ac-e6185db3a5a1) Of course this is probably a contrived case, but I just wanted to exaggerate the effect to make my point more clear. Probably nothing that we could fix with some themeing, but I wanted to check in what other are feeling.
Member

I did a version last week with the cache line under the scrub area:

Looks good! +1

when I zoom out, I feel like it kinda blends into the the rest of the UI
Probably nothing that we could fix with some themeing, but I wanted to check in what other are feeling.

It doesn't bother me tbh. In a screenshot it might look a bit confusing but while actually using it the cache line is fixed in place, so it's pretty obvious what is Blender's UI and what not because it zooms in/out with the rest of the content in the timeline.

> I did a version last week with the cache line under the scrub area: Looks good! +1 > when I zoom out, I feel like it kinda blends into the the rest of the UI > Probably nothing that we could fix with some themeing, but I wanted to check in what other are feeling. It doesn't bother me tbh. In a screenshot it might look a bit confusing but while actually using it the cache line is fixed in place, so it's pretty obvious what is Blender's UI and what not because it zooms in/out with the rest of the content in the timeline.
Sebastian Parborg force-pushed vse_cache_overlay from 3c87ecbbde to 7f71deb4ed 2024-04-29 15:22:00 +02:00 Compare
Member

Just tested it, visually it works. How do you turn it on though?

Tested on factory startup. The only way I found to enable the cache is by:

  1. Enable Developer Extras
  2. Enable Show Cache from the ViewCache menu.
  3. Enable Final Images from the Cache menu.


The Show Cache entry in the View menu should be always available regardless of Developers Extras, and turned on by default. All the options inside the Cache menu I'm not sure I leave it to you developers to decide. But the Final Images option should be on by default at least? Otherwise it takes opening the menu twice just to see anything.

Just tested it, visually it works. How do you turn it on though? Tested on factory startup. The only way I found to enable the cache is by: 1. Enable `Developer Extras` 2. Enable `Show Cache` from the `View` → `Cache` menu. 3. Enable `Final Images` from the `Cache` menu. <video src="/attachments/f2450d45-1f58-44d0-a9ca-e1027d723a97" title="vse_enable_cache_line.mov" controls></video> ---- The `Show Cache` entry in the `View` menu should be always available regardless of `Developers Extras`, and turned on by default. All the options inside the `Cache` menu I'm not sure I leave it to you developers to decide. But the `Final Images` option should be on by default at least? Otherwise it takes opening the menu twice just to see anything.
Author
Member

Talked a bit more with Pablo.
We will move the "Developer Extras" Cache panel to the n-panel so that both the cache drawing and settings is in the same place.

Without developer extras, only the "Final images" view toggle will be available and it will no longer be implicitly tied to the overlay "Show cache" option when not in "Developer Extras" mode.

Talked a bit more with Pablo. We will move the "Developer Extras" Cache panel to the n-panel so that both the cache drawing and settings is in the same place. Without developer extras, only the "Final images" view toggle will be available and it will no longer be implicitly tied to the overlay "Show cache" option when not in "Developer Extras" mode.
Member

Okay I missed the fact that Cache is now an Overlay setting.

Thinking about it again, these options don't belong in the View menu. All Cache settings have their own tab on the sidebar, and their own panel. Display settings should be under a Display subpanel of the Cache Settings panel.

Cache in View menu

The new Display panel should be always open (there are so few panels in the sidebar anyway), and always available regardless of Developer Extras being on. No need to hide away settings that are considered important. If it can be toggled (by the setting in the parent panel), its display should be toggleable too.

If cache types like Composite or Preprocessed are too advanced/technical/debug they could be hidden via Developer Extras, but not Final or Raw perhaps?

Mockup:

Display panel mockup

There is a checkbox in the header that basically toggles the show_cache overlay. Also sorted the cache types list so the most important for the user (Final) is first.

Okay I missed the fact that `Cache` is now an Overlay setting. Thinking about it again, these options don't belong in the `View` menu. All `Cache` settings have their own tab on the sidebar, and their own panel. Display settings should be under a `Display` subpanel of the `Cache Settings` panel. ![Cache in View menu](/attachments/791d79d2-b139-4867-9551-6b57f0379829) The new `Display` panel should be always open (there are so few panels in the sidebar anyway), and always available regardless of `Developer Extras` being on. No need to hide away settings that are considered important. If it can be toggled (by the setting in the parent panel), its display should be toggleable too. If cache types like `Composite` or `Preprocessed` are too advanced/technical/debug they could be hidden via `Developer Extras`, but not Final or Raw perhaps? Mockup: ![Display panel mockup](/attachments/06ba5790-649c-4239-9a67-cf970e4c181c) There is a checkbox in the header that basically toggles the `show_cache` overlay. Also sorted the cache types list so the most important for the user (Final) is first.
Member

Awesome! Works as expected now.

One last thing. Since cache is off by default in old files, it might be a good idea to at least do versioning to turn on Final cache on old files. Otherwise enabling the overlay doesn't seem to do anything unless the user goes to the sidebar and enables Final manually. A bit cumbersome.

How it looks at the moment on an older file:

cache settings panel

Other than that I'd say it's good to go.

Awesome! Works as expected now. One last thing. Since cache is off by default in old files, it might be a good idea to at least do versioning to turn on `Final` cache on old files. Otherwise enabling the overlay doesn't seem to do anything unless the user goes to the sidebar and enables `Final` manually. A bit cumbersome. How it looks at the moment on an older file: ![cache settings panel](/attachments/0ebe7356-7737-4f7e-b6a5-6c7f727c6d08) Other than that I'd say it's good to go.
Richard Antalik approved these changes 2024-04-30 05:53:02 +02:00
Pablo Vazquez approved these changes 2024-04-30 11:35:47 +02:00

I think it will be indeed very good to enable Final cache display for the current files. From user perspective it was a hidden option, which was disabled by default. And with the current state of PR there is the visibility that an overlay option does nothing.

I think it will be indeed very good to enable Final cache display for the current files. From user perspective it was a hidden option, which was disabled by default. And with the current state of PR there is the visibility that an overlay option does nothing.
Sergey Sharybin reviewed 2024-04-30 12:29:34 +02:00
@ -829,3 +826,1 @@
SEQ_CACHE_VIEW_PREPROCESSED = (1 << 7),
SEQ_CACHE_VIEW_COMPOSITE = (1 << 8),
SEQ_CACHE_VIEW_FINAL_OUT = (1 << 9),
/* Deprecated: CACHE_VIEW setting has been moved to CACHE_SHOW in DNA_space_types.h

For those it works better to clear the flag now and rename them to SEQ_CACHE_UNUSED_{5,6,7,8,9}. See, for example, WO_MODE_UNUSED_1 = 1 << 1, /* cleared */.

For those it works better to clear the flag now and rename them to `SEQ_CACHE_UNUSED_{5,6,7,8,9}`. See, for example, `WO_MODE_UNUSED_1 = 1 << 1, /* cleared */`.

No need to refer to something that was there in the code, but not any more. The comments are about the current state of the code.

No need to refer to something that was there in the code, but not any more. The comments are about the current state of the code.
ZedDB marked this conversation as resolved
Sebastian Parborg force-pushed vse_cache_overlay from d407694e4c to 23a510c273 2024-05-01 14:11:15 +02:00 Compare
Author
Member

@blender-bot build

@blender-bot build
Author
Member

The lint and windows faliures seems unrelated to my changes from what I can see...

The lint and windows faliures seems unrelated to my changes from what I can see...
Sergey Sharybin approved these changes 2024-05-01 15:53:04 +02:00
Sergey Sharybin left a comment
Owner

Indeed I don't think the failed tests are anything to do with this PR.

I've left a small comment about code comment, which, I think does not really help and better be removed for clarity. The rest seems to work as expected to me now.

So if you can remove that comment, think you can go ahead and land. No need to wait an extra iteration of review for that,

Indeed I don't think the failed tests are anything to do with this PR. I've left a small comment about code comment, which, I think does not really help and better be removed for clarity. The rest seems to work as expected to me now. So if you can remove that comment, think you can go ahead and land. No need to wait an extra iteration of review for that,
Sebastian Parborg force-pushed vse_cache_overlay from 6a0758e294 to 5cb1385b3c 2024-05-02 10:47:40 +02:00 Compare
Author
Member

@blender-bot build

@blender-bot build
Sebastian Parborg added 1 commit 2024-05-02 15:36:34 +02:00
buildbot/vexp-code-patch-lint Build done. Details
buildbot/vexp-code-patch-darwin-x86_64 Build done. Details
buildbot/vexp-code-patch-windows-amd64 Build done. Details
buildbot/vexp-code-patch-linux-x86_64 Build done. Details
buildbot/vexp-code-patch-darwin-arm64 Build done. Details
buildbot/vexp-code-patch-coordinator Build done. Details
d2a7e8bfc4
Attempt to fix versioning segfault
Author
Member

@blender-bot build

@blender-bot build
Sebastian Parborg merged commit 221951657b into main 2024-05-02 16:36:21 +02:00
Sebastian Parborg deleted branch vse_cache_overlay 2024-05-02 16:36:24 +02:00
Sign in to join this conversation.
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
5 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: blender/blender#119428
No description provided.