Add shift color index tool #887
Conversation
|
One idea to make it more powerful is to use first and secondary colors as a way to set boundary of index cycling - so we can limit the effect only to specified color range in the palette |
|
I added ability to set colour ranges for colour cycling based on what the user sets to primary and secondary colour. The palette tool will display it only when the current tool is the new index shift brush when you set ranges like this - the brush tool will use the boundaries and prevent things like cycling from cloth colours into skin colours. you can also use it to exclude the first few colours if you want. In my example I am excluding black (line colours) and white from being affected by the brush index tool |
|
I want to add a bunch of other features to piskel to manipulate colours, but will not proceed before seeing if I can get a feature merged |
|
Here is trying to do shading with the Lighten/Darken tool: Problems:
Doing shading with the tool in this PR:
|
If your improvements aren't merged, maybe you could consider packaging your version of Piskel as a Chrome App (cf. https://github.com/Archxiao/cros-piskel) to distribute it to users. |
|
Well, if months pass by with no answer from the author, I guess I can just fork it and call it piskel-plus or something like that :) then implement a bunch of features I want myself and are requested here |
|
In any case I would very much appreciate feedback from the original author. My main reasoning for implementing this feature is the need for a better shading workflow. There are some areas in which piskel can really do with some improvements. Color palette management and shading workflows come up on top of my list |
|
I kind of wanted to get this feature to the version of piskel that is running in gdevelop. I guess I can update gdevelop to use my fork instead of this abandoned looking main branch, but the problem of having noone review stuff on the main branch will remain. The majority of piskel users (outside of gdevelop) will not get this feature |
|
@blurymind hi! So original author here. Sorry about the lack of feedback, as you can probably tell I am not doing a great job at maintaining this project anymore. Was trying to keep an eye on PRs and issues for a while, but looks like I failed to do that as well. In any case, the feature looks great, I will try to review the code, but feel free to fork and release your version in another way if I go silent again :/ Sorry about that 🙏 |
|
hi @juliandescottes no worries :) Would you like me to submit a PR with all of those in one or do you prefer a pr per feature? I would very much prefer to keep piskel one project and not split the community. The author of Gdevelop, @4ian is telling me that it is getting quite a lot of usage in GD atm (we have analytics in the app that shows us that it is!) So we want to very much keep it well and alive. |
|
Hey @blurymind and @juliandescottes,
Yep I checked the numbers and there is 5k launch of the editor every day by a bit less than 1 thousand unique user every day. Piskel is being really useful and used by GDevelop users to create their game! |
|
@juliandescottes some of these users have started reporting piskel related bugs to @4ian and on the gdevelop tracker, but without anyone to review/approve fixes here, I am sort of forced to keep pushing them to my fork. |
@4ian
@blurymind Who knows, maybe that will motivate me to go back to this project as well (working alone on something can get pretty tiring :) ) |
|
@juliandescottes thank you for looking into these :) I hope I havent missed anything that is in In any case one of the reasons we picked piskel for GD is that it is still the best html5 pixel editor .. its a worthy cause to keep it alive and well! |
There was a problem hiding this comment.
Thanks @blurymind ! The code looks really good, I have a few comments here and there but nothing major. I am really impressed that you managed to add a feature on this undocumented and very custom project! I also appreciate that you followed the conventions in place in the rest of the codebase.
I have some thoughts and questions on the feature itself, and how it fits in the current product.
The feature was hard to understand in the first place. I think right now it is a bit too complex to be enabled by default in the version of Piskel that I push to piskelapp.com. Many users of piskelapp.com are kids & beginners. I suppose that the users of GDevelop are more advanced users?
I don't think any of this should block landing your PR though. The change is not really intrusive, so we can decide the next steps and handle them in dedicated issues/PRs. Either find a way to make the feature easier to use, or we make it configurable so that GDevelop can enable it by default.
I want to explain a bit more what felt difficult for me:
- unless you have a correct setup in the palette the tool does nothing when clicking on the sprite, so I thought it was broken initially
- I didn't understand the logic that made the "cycling/blue" icons appear. I think I clicked randomly on the palette for a few minutes without getting anywhere
- when I understood that the cycled color were marked according to the gap between the primary and secondary color, I was still not sure how an index was mapped to its "shift" index
Finally I understood that primary was mapped to secondary (and so on for the blue ones). So I realized that the tool was not only relying on having a good palette, but also on the order of colors in the palette, and on having consistent gaps between shades of colors. Seemed like a lot to me especially considering how painful it is to manage palettes in Piskel.
I'm interested to know if the current UX of the tool is temporary and you already have ideas to make it more user friendly?
src/js/tools/drawing/ShiftIndex.js
Outdated
| this.superclass.constructor.call(this); | ||
|
|
||
| this.toolId = 'tool-shift-index'; | ||
| this.helpText = 'ShiftIndex'; |
There was a problem hiding this comment.
The helpText becomes the title of the tool's tooltip, so maybe this should be Shift Index?
There was a problem hiding this comment.
lets call it Shift Palette Color Index Brush - it better hints at how its used
src/js/tools/drawing/ShiftIndex.js
Outdated
| this.helpText = 'ShiftIndex'; | ||
| this.shortcut = pskl.service.keyboard.Shortcuts.TOOL.SHIFT_INDEX; | ||
|
|
||
| this.tooltipDescriptors = [{ key: 'ctrl', description: 'Shift Index backwards' }]; |
There was a problem hiding this comment.
We could also add a general description, not linked to a specific modifier
this.tooltipDescriptors = [
{
description: 'Replace pixels using the primary color by the secondary color. ' +
'Note that both colors need to be available in the currently selected palette ' +
'and the primary color needs to have a lower index than the secondary color.'
},
{ key: 'ctrl', description: 'Shift Index backwards' },
];(overall I'm not sure how to convey the complexity of the feature in short and concise manner, so the text here is really just a placeholder)
There was a problem hiding this comment.
Promotion describes is as follows
Shade
Shifts the image colors which are part of the current gradient. So for each color the next or previous color within the gradient will be used depending on left or right click. This way you can shade objects to emphasize their structure, to create different color effects or to smooth edges.
https://www.cosmigo.com/promotion/docs/onlinehelp/MenuMode.htm
There was a problem hiding this comment.
how about
this.tooltipDescriptors = [
{
description: 'Shifts the colors which are a part of the current palette. ' +
'For each pixel color it touches, the next or previous color within the palette will be used. ' +
'Use the primary and secondary colors in the palette to set ' +
'cell shade range boundaries. For example if your palette has: ' +
'- light red, dark red, light blue, dark blue - ' +
'you would want to set the primary colour to light red and secondary to dark red. ' +
'This will prevent the brush from cycling red to blue'
},
{ key: 'ctrl', description: 'Shift Index backwards' },
];
|
Thank you for reviewing this @juliandescottes It's true that this feature is a sort of an advanced feature, that very few pixel apps have. Piskel's implementation here is actually a much simplified and much easier to use design from the one in pro-motion NG. I believe that some of Piskel's users are actually more experienced than we give them credit for, so I believe that they will make very good use of this if we let them so. Would you be more inclined to have it enabled if I make a tutorial video on how to use it? :) or write in a wiki somewhere? |
|
@blurymind I'll try to take a look at pro-motion NG to see how the UX is handled, thanks for the pointer! (and if you have any other software you take inspiration from, don't hesitate to mention them, it will help if we have a common frame of reference)
You might be right, and maybe I am overly worried with keeping Piskel simple. Most of the actual feedback I receive comes from Piskel usage in schools. So keeping it easy to use is really important for me, so that the userbase I know about can keep using Piskel easily. But no doubt some users are probably more advanced.
Maybe :) So, I think my criteria is: it needs to be easy to pick up for a new user who has not heard about this feature. The UI should help as much as possible, and if more is needed then link to a guide/video. For instance we could show the palette differently so that the user has more control over which color will shift to which color, and that the relationship between the colors is clearer. But this will require more thoughts and work, so I think the easiest will be to merge this as is (after the current review comments have been addressed) |
|
Ok good idea! :) In that case I will apply all review notes and after this gets in, I will make a tutorial video on how to use it. |
You mean a wiki for users, and not contributors, right? No I don't have anything :) I did create a repo at that time... https://github.com/piskelapp/docs but it's really just an empty repo :) We can start by adding a section in the wiki, or add md files in a docs/ folder in piskel? |
|
@juliandescottes I applied all review notes :) Let me know if anything else needs changing. I updated the tooltip of the tool to better explain its use, it should hopefully make it clearer. Not sure why travis test is failing me, I applied the npm run format command |
|
FYI @blurymind I just added this PR to my fork of Piskel, which has had some changes made to future proof it building and running, a handful of quality of life improvements, and some additional tools included. |
|
@gingerbeardman if its of any use, I believe I had all of my improvements on piskel on this branch https://github.com/blurymind/piskel/tree/piskel-plus was thinking of turning it into its own project |
|
I'll check 'em out! thanks @blurymind |
This adds a new brush tool to piskel - it can be used to shift the color index of pixel colors either in the positive or negative direction so one can quickly shade pixel art if the color index of the palette is set well.
You can set colour ranges for colour cycling based on what the user sets to primary and secondary colour. The palette tool will display it only when the current tool is the new index shift brush
when you set ranges like this - the brush tool will use the boundaries and prevent things like cycling from cloth colours into skin colours. you can also use it to exclude the first few colours if you want. In my example I am excluding black (line colours) and white from being affected by the brush index tool
An example of such tool can be found in professional pixel art software such as cosmigo pro-motion
Here is the request describing how it can be used
#863
It operates on the active palette
The main advantages of this tool over Lighten/Darken: