-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Fix for Support ARGB for pen #393 #403
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
Fix for Support ARGB for pen #393 #403
Conversation
Requires fix to scratch-render before this will work (see: 6f5acfee7b961d71237f1fd50bb3d5a5139c527e)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Two very small lint issues that are failing CI. You should be able to test this locally by running npm test.
src/blocks/scratch3_pen.js
Outdated
| penState.penAttributes.color4f[1] = rgb.g / 255.0; | ||
| penState.penAttributes.color4f[2] = rgb.b / 255.0; | ||
| if (rgb.hasOwnProperty('a')) { // Will there always be an 'a'? | ||
| penState.penAttributes.color4f[3] = rgb.a/255.0; |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
src/util/color.js
Outdated
| var g = (decimal >> 8) & 0xFF; | ||
| var b = decimal & 0xFF; | ||
| return {r: r, g: g, b: b}; | ||
| return {r: r, g: g, b: b, a: a>0 ? a : 255}; |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
Had to update the test scripts to handle the alpha channel, also I note that all the hex tests are using CSS notation, not scratch notation (which is 0x not #)
|
Ok, I've fixed the lint issues - I also had to add to the test scripts to handle the new alpha values. Do I need to do anything to move the pull request forward? |
|
Thanks @griffpatch ! |
…esize-bug Show workspace before triggering resize
Requires fix to scratch-render before this will work.
See:
6f5acfee7b961d71237f1fd50bb3d5a5139c527e