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

Add glitch0r plugin #7

Merged
merged 2 commits into from Dec 5, 2016

Conversation

Projects
None yet
3 participants
@namikiri
Copy link
Contributor

commented Nov 30, 2016

I love glitches. Glitches love me. Why we shouldn't make a nice effect for such an interesting framew0rk?

Thank you for inspiration, frei0r maintainers. The video on frei0r.dyne.org made me happy and inspired me to write this little plugin.

Hope this will be helpful :3

@jaromil

This comment has been minimized.

Copy link
Member

commented Dec 3, 2016

hi, thanks for the contribution! can you rebase all commits into a single one?

@namikiri

This comment has been minimized.

Copy link
Contributor Author

commented Dec 3, 2016

@jaromil Yes, of course. Just did it.

I am new to Github community and I am planning to create another plugin, should I rebase further commits again before making a new pull req?

@ddennedy

This comment has been minimized.

Copy link
Collaborator

commented Dec 3, 2016

The GitHub web UI makes it easy to squash commits or rebase and merge. However, your rebase does make it easier to review and add comments to lines.

#include "frei0r.h"

/* cheap & fast randomizer (by Fukuchi Kentarou) */
uint32_t randval;

This comment has been minimized.

Copy link
@ddennedy

ddennedy Dec 3, 2016

Collaborator

Please make all private symbols static. Names such as fastrand and fastsrand have a strong chance of collision.

break;

case 2 : // add some unneeded colors
*(pixel) = distortionSeed | *(pixel);

This comment has been minimized.

Copy link
@ddennedy

ddennedy Dec 3, 2016

Collaborator

These pixel operations combined are not really endian-safe because the hex value 0x00ffffff for F0R_COLOR_MODEL_RGBA8888 only makes alpha clear on little endian. If you do not want to affect the alpha channel, then you should cast the pixel pointer to uint8_t* p, save alpha=p[3], and then restore it as p[3]=alpha. Then, you can make the color distortion ranges in glitch0r_state_reset() from 0 - 0xffffffff. Does this make sense?

switch(param_index) {
case 0:
{
info->name = "GlitchFrequency";

This comment has been minimized.

Copy link
@ddennedy

ddennedy Dec 3, 2016

Collaborator

This is not a big deal, but some frei0r plugin hosts expose these names as labels in the UI. Thus, spaces are preferred over PascalCase.

@ddennedy

This comment has been minimized.

Copy link
Collaborator

commented Dec 3, 2016

Thank you for your contribution, @namikiri !

@namikiri

This comment has been minimized.

Copy link
Contributor Author

commented Dec 4, 2016

Thank you @ddennedy for your review!
I've fixed everything that you pointed in my code.

First time I thought that glitching means total data distortion, but then I tried saving alpha channel and it really makes sense. Thank you for pointing me on it.

Should I squash this commit too?

@ddennedy ddennedy merged commit 79239c5 into dyne:master Dec 5, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.