Animation: Keep icons aligned when curves are baked #108518

Merged

Christoph Lendenfeld merged 6 commits from ChrisLend/blender:channel_draw_lock_on_baked into main

2023-09-05 14:27:19 +02:00

Christoph Lendenfeld commented

2023-06-01 15:10:34 +02:00

Member

When curves were baked, the lock icon wasn't drawn, which resulted in a misalignment of the other icons.

With this patch, the icons are kept in alignment.

Also removes the comments that are no longer valid.

When curves were baked, the lock icon wasn't drawn, which resulted in a misalignment of the other icons. ![image](/attachments/2ba58d0d-791f-4ef0-a14c-d465af15b781) With this patch, the icons are kept in alignment. ![image](/attachments/4c972649-72e3-4e61-b599-423d7d98fa65) Also removes the comments that are no longer valid.

6.1 KiB

6.7 KiB

Christoph Lendenfeld added the

Animation & Rigging

label 2023-06-01 15:10:34 +02:00

Christoph Lendenfeld added 1 commit 2023-06-01 15:10:40 +02:00

d3630a7946 draw lock on baked curves

Christoph Lendenfeld requested review from Sybren A. Stüvel 2023-06-01 15:11:22 +02:00

Christoph Lendenfeld requested review from Harley Acheson 2023-06-01 15:11:29 +02:00

Christoph Lendenfeld added the

label 2023-06-01 15:11:41 +02:00

Sybren A. Stüvel reviewed 2023-06-12 15:50:28 +02:00

source/blender/editors/animation/anim_channels_defines.c Outdated

													
				@ -5185,6 +5177,12 @@ static void draw_setting_widget(bAnimContext *ac,

				      UI_but_disable(but, TIP_("Can't edit this property from a linked data-block"));

				    }

				  }

				  if (ale->datatype == ALE_FCURVE && setting == ACHANNEL_SETTING_PROTECT) {

Sybren A. Stüvel commented

2023-06-12 15:50:28 +02:00

Member

Thanks for the inspiration, I used it as inspiration for something similar in #106214 :)

That PR has a little different structure though, so that it's a little easier to extend for other settings. It would duplicate some of the code (the if (is fcurve) and the cast), but IMO that's a fair trade-off for keeping the per-settings-switch structure of the rest of the code.

Thanks for the inspiration, I used it as inspiration for something similar in #106214 :) That PR has a little different structure though, so that it's a little easier to extend for other settings. It would duplicate some of the code (the `if (is fcurve)` and the cast), but IMO that's a fair trade-off for keeping the per-settings-`switch` structure of the rest of the code.

dr.sybren marked this conversation as resolved

Sybren A. Stüvel commented

2023-06-12 15:55:11 +02:00

Member

I am wondering if disabling the button is even needed. On the one hand, locking doesn't have an effect on baked FCurves, on the other hand it also doesn't break anything.

We could use the 'inactive' state here, like I did in #106214 ?

> I am wondering if disabling the button is even needed. On the one hand, locking doesn't have an effect on baked FCurves, on the other hand it also doesn't break anything. We could use the 'inactive' state here, like I did in #106214 ?

Christoph Lendenfeld added 1 commit 2023-06-14 10:08:22 +02:00

043be11c45 Merge branch 'main' into channel_draw_lock_on_baked

Christoph Lendenfeld commented

2023-06-14 10:10:38 +02:00

Author

Member

@dr.sybren I used the inactive state but then found out that the lock state does make a difference, even on baked curves
it's about the modifiers. If the curve is locked you can't add any
So I chose to keep the lock icon active

The only downside is that there is no immediate way to tell in the channel box if a curve is baked. But I am wondering how important that is

@dr.sybren I used the inactive state but then found out that the lock state does make a difference, even on baked curves it's about the modifiers. If the curve is locked you can't add any So I chose to keep the lock icon active The only downside is that there is no immediate way to tell in the channel box if a curve is baked. But I am wondering how important that is

Christoph Lendenfeld added 1 commit 2023-06-14 10:11:16 +02:00

6352b11745 lock button no longer disabled

Harley Acheson commented

2023-07-20 00:38:08 +02:00

Member

Some general comments. I might be misunderstanding here

You are trying to fix this alignment problem by showing the lock icon even when it isn't strictly needed. It looks like you tried setting it inactive and disabling and those had bad effects. Have you tried just filling the space? Ie, adding blank space, padding, or ICON_NONE to the place where the lock would be. I would just try making it always valid in acf_fcurve_setting_valid but then where it is shown in ANIM_channel_draw_widgets (protect... section) don't do the drawing (but allow the change in offset) under whatever conditions fit (like !(FCurve)ale.data->bezt).

Note that in acf_fcurve_setting_valid the default return is true. So you can just remove the entire ACHANNEL_SETTING_PROTECT section rather than always return true in it.

Some general comments. I might be misunderstanding here You are trying to fix this alignment problem by showing the lock icon even when it isn't strictly needed. It looks like you tried setting it inactive and disabling and those had bad effects. Have you tried just filling the space? Ie, adding blank space, padding, or ICON_NONE to the place where the lock would be. I would just try making it always valid in `acf_fcurve_setting_valid` but then where it is shown in `ANIM_channel_draw_widgets` (protect... section) don't do the drawing (but allow the change in offset) under whatever conditions fit (like !(FCurve)ale.data->bezt). Note that in `acf_fcurve_setting_valid` the default return is true. So you can just remove the entire ACHANNEL_SETTING_PROTECT section rather than always return true in it.

Christoph Lendenfeld added 2 commits 2023-07-28 10:25:26 +02:00

c3c16a48df Merge branch 'main' into channel_draw_lock_on_baked

371aa6bbbc don't draw lock icon on baked curves but keep offset

Christoph Lendenfeld changed title from ~~Animation: Draw lock icon on baked curves~~ to Animation: Keep icons aligned when curves are baked

2023-07-28 10:28:21 +02:00

Christoph Lendenfeld added 1 commit 2023-07-28 10:31:50 +02:00

1c8ff888a9 typo

Christoph Lendenfeld commented

2023-07-28 10:34:04 +02:00

Author

Member

Thanks for the suggestion @Harley
We discussed it in yesterdays A&R module meeting and decided to go along with that.
So I updated the code and the PR description

Thanks for the suggestion @Harley We discussed it in yesterdays A&R module meeting and decided to go along with that. So I updated the code and the PR description

Harley Acheson approved these changes 2023-08-22 00:20:35 +02:00

Harley Acheson left a comment

Member

Looks great.

Looks great.

Harley Acheson commented

2023-08-22 00:25:20 +02:00

Member

@ChrisLend - Sorry for my delay in looking at this again. I did get a notification of your update on July 28 but somehow let it fall off my plate.

To make matters worse this really should be merged before Wednesday (Aug 23), so hopefully you can get a final "looks good!" from
Sybren fairly soon. Just blame me for any rush.

@ChrisLend - Sorry for my delay in looking at this again. I did get a notification of your update on July 28 but somehow let it fall off my plate. To make matters worse this really should be merged before Wednesday (Aug 23), so hopefully you can get a final "looks good!" from Sybren fairly soon. Just blame me for any rush.

Harley Acheson commented

2023-08-23 21:45:39 +02:00

Member

To make matters worse this really should be merged before Wednesday (Aug 23)...

Um... my comment about urgency was based on my incorrect understanding about the release schedule. So ignore that.

> To make matters worse this really should be merged before Wednesday (Aug 23)... Um... my comment about urgency was based on my incorrect understanding about the release schedule. So ignore that.

Christoph Lendenfeld commented

2023-08-24 11:16:25 +02:00

Author

Member

Um... my comment about urgency was based on my incorrect understanding about the release schedule. So ignore that.

I think sybren is on holiday now, so will only be approved after. This should be good to land in BCon2, it's not strictly speaking a new feature

> Um... my comment about urgency was based on my incorrect understanding about the release schedule. So ignore that. I think sybren is on holiday now, so will only be approved after. This should be good to land in BCon2, it's not strictly speaking a new feature

Harley Acheson commented

2023-08-24 17:01:23 +02:00

Member

I think sybren is on holiday now, so will only be approved after. This should be good to land in BCon2, it's not strictly speaking a new feature

For sure. I was just confused. As usual. LOL.

> I think sybren is on holiday now, so will only be approved after. This should be good to land in BCon2, it's not strictly speaking a new feature For sure. I was just confused. As usual. LOL.

Sybren A. Stüvel approved these changes 2023-09-05 14:21:31 +02:00

Christoph Lendenfeld merged commit b7a84c49ea into main

2023-09-05 14:27:19 +02:00

Christoph Lendenfeld referenced this issue from a commit

2023-09-05 14:27:20 +02:00

Animation: Keep icons aligned when curves are baked

Christoph Lendenfeld deleted branch channel_draw_lock_on_baked

2023-09-05 14:27:21 +02:00

Stefan Werner referenced this issue from a commit

2023-09-07 15:02:39 +02:00

Animation: Keep icons aligned when curves are baked

Sign in to join this conversation.

No reviewers

No Label

Animation & Rigging

Asset Browser Project Overview

Automated Testing

Blender Asset Bundle

Dependency Graph

Development Management

EEVEE & Viewport

Images & Movies

Motion Tracking

Nodes & Physics

Pipeline, Assets & IO

Platforms, Builds & Tests

Render & Cycles

Render Pipeline

Sculpt, Paint & Texture

Video Sequencer

Virtual Reality

Blender 2.8 Project

Milestone 1: Basic, Local Asset Browser

Good First Issue

Animation & Rigging

Development Management

EEVEE & Viewport

Nodes & Physics

Pipeline, Assets & IO

Platforms, Builds & Tests

Render & Cycles

Sculpt, Paint & Texture

Needs Info from Developers

Needs Information from User

No Milestone

No project

No Assignees

3 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#108518

No description provided.