Skip to content

Add VisibleWhen and EnabledWhen attributes to SimpleEffectDialog#1960

Merged
cameronwhite merged 6 commits intoPintaProject:masterfrom
Matthieu-LAURENT39:SimpleEffectDialog-condition
Feb 1, 2026
Merged

Add VisibleWhen and EnabledWhen attributes to SimpleEffectDialog#1960
cameronwhite merged 6 commits intoPintaProject:masterfrom
Matthieu-LAURENT39:SimpleEffectDialog-condition

Conversation

@Matthieu-LAURENT39
Copy link
Contributor

@Matthieu-LAURENT39 Matthieu-LAURENT39 commented Jan 29, 2026

This adds two new attributes for use with SimpleEffectDialog: VisibleWhen and EnabledWhen.
These allow hiding or disabling a field based on a condition that will be rechecked whenever EffectData changes.

This was started for use with #1815, but other effects should be able to benefit from it.
I went through the current list of effects and retrofitted it on the existing ones, so this PR also includes that.

Example from one of the updated effects (Voronoi Diagram):

Voronoi Diagram dialog, with many options Voronoi Diagram dialog, with some options hidden

This allows attributes in SimpleEffectDialog to automatically set their enabled state/visibility based on a condition
@Matthieu-LAURENT39 Matthieu-LAURENT39 force-pushed the SimpleEffectDialog-condition branch from 377baba to ff842ab Compare January 29, 2026 21:01
@Matthieu-LAURENT39
Copy link
Contributor Author

Matthieu-LAURENT39 commented Jan 29, 2026

Note that I originally wanted to use lambdas in the attributes, but C# doesn't seem to support lambdas inside attributes.
I therefore instead went with reflection, which seems to be the idiomatic way in C#. But given that I'm still fairly new to C#, I'm not too sure if that's the best approach.
If it's not, feel free to point it out, I'll gladly use another approach if a better one is available.

I'm also not sure if I used reflectors properly since it was my first time using them, so I'd appreciate feedback on that part 😁

@Matthieu-LAURENT39 Matthieu-LAURENT39 force-pushed the SimpleEffectDialog-condition branch from 27c67f4 to 8382d2d Compare January 29, 2026 21:35
@cameronwhite
Copy link
Member

This is great!
The approach looks reasonable to me but I'll also ask @Lehonti to look over this too (who has done a lot of the recent work on the effect dialog)

A couple minor suggestions / thoughts:

  • Could move EvaluateCondition() into ReflectionHelper.cs to keep it with other reflection utilities
  • Since this reflection is done a lot more frequently than the initial dialog creation (e.g. while dragging effect parameters around), it might be worthwhile to do the reflection once up front and create delegates to store in ConditionalWidget. Although I'm not sure offhand how much this will matter performance-wise vs evaluating the updated effect :)

This also makes CreateConditionDelegate (and EvaluateCondition) hard-fail if the proprety/method is missing
@Matthieu-LAURENT39
Copy link
Contributor Author

Matthieu-LAURENT39 commented Jan 30, 2026

* Since this reflection is done a lot more frequently than the initial dialog creation (e.g. while dragging effect parameters around), it might be worthwhile to do the reflection once up front and create delegates to store in `ConditionalWidget`. Although I'm not sure offhand how much this will matter performance-wise vs evaluating the updated effect :)

I had tested on a weak machine that changing values fast on effects using VisibleWhen doesn't cause slowdowns, but using delegates doesn't increase code complexity, so I went with it as it's an easy performance gain

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! This can actually go a step further - I should've mentioned this more clearly in my last comment :)

You can use MethodInfo.CreateDelegate () , which creates a strongly-typed delegate that's more like calling the method directly performance-wise

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to know, thanks!
I updated the implementation to use CreateDelegate

@cameronwhite
Copy link
Member

Looks good , thanks!

@cameronwhite cameronwhite merged commit ba09251 into PintaProject:master Feb 1, 2026
7 checks passed
@Lehonti
Copy link
Contributor

Lehonti commented Feb 1, 2026

Hmm @cameronwhite is right, MethodInfo.Invoke is slow. I am a bit out of the loop, but I faintly remember doing something suboptimal and the reflection method I was using created the delegate but not in the way one would expect (like requiring extra arguments or something like that). If you guys were able to verify that CreateDelegate works, then I think @Matthieu-LAURENT39 solved that issue that I had left lingering. But I will review this later so that I can give you a more informed opinion

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Comments