Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce the flashing in ext_event and identity examples #782

Merged
merged 3 commits into from
Apr 2, 2020

Conversation

futurepaul
Copy link
Collaborator

This is pretty hacky, let me know if you have a better idea! My main idea is to move only slightly toward the new random color. The result is the color stays pretty subtle, but I didn't want to introduce more state.

@futurepaul futurepaul added the S-needs-review waits for review label Mar 30, 2020
Copy link
Member

@cmyr cmyr left a comment

Choose a reason for hiding this comment

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

heh I thought this was just going to turn slow down the timer 😆

Copy link
Member

@cmyr cmyr left a comment

Choose a reason for hiding this comment

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

I'm not sure how I feel.

I think it's interesting and I think it looks nicer, but it feels vaguely less illustrative? But then these examples weren't very illustrative, initially.

So... I'm going to say that you should do whatever you want here that feels right, and merge when you're happy.

Copy link
Collaborator

@fishrockz fishrockz left a comment

Choose a reason for hiding this comment

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

This looks really good, I will go verify, and am happy to approve with the changed function name

druid/examples/identity.rs Outdated Show resolved Hide resolved
@fishrockz
Copy link
Collaborator

fishrockz commented Mar 31, 2020

I get @cmyr 's reservation and I expected a change to just turn the frequency down too. But the effect of this is actually quite nice as you still get the immediate change and you still get a reasonable range of colours, if anything i think the filter could let through slightly higher frequency change.

@futurepaul
Copy link
Collaborator Author

Okay this is a new approach that gets rid of all the old random color logic and just mutates the color randomly. Hits a wider range of colors, still is low on flashing (my primary concern). I'll merge this later today if there are no objections.

@futurepaul futurepaul merged commit 56a7488 into linebender:master Apr 2, 2020
@xStrom xStrom removed the S-needs-review waits for review label May 15, 2020
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.

4 participants