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

Inconsistent palette import behavior #1174

Open
scribblemaniac opened this Issue Feb 12, 2019 · 10 comments

Comments

Projects
None yet
4 participants
@scribblemaniac
Copy link
Member

scribblemaniac commented Feb 12, 2019

Issue Summary

Depending on which palette file format is used, the new palette may append or overwrite the existing palette.

Actual Results

When importing a Pencil2D palette (.xml), the existing palette will be replaced by the new palette. When importing a GIMP palette (.gpl), the existing palette will remain and the new palette will be added to the bottom.

Expected Results

I would expect the behavior to be consistent regardless of the file format being imported.

Steps to reproduce

Here are two palettes to make reproducing the bug easier: testpalettes.zip

  1. Create a new project
  2. Import the testpalette.gpl file
  3. Observe how the Vivid Purple color has been added to the end of the default palette.
  4. Create a new project
  5. Import the testpalette.xml file
  6. Observe how the default palette colors have been removed and replaced with the Vivid Purple color.

System Information

  • Pencil2D Version: January 6th Nightly Build (0dde212)

  • Operating System: macOS 10.13.6

  • RAM Size: 16 GB

  • Graphics Tablet: None


This issue shouldn't be too hard to fix, but we need to discuss what behavior is desired. Personally I think the current gpl behavior is best because then you can easily import multiple palettes for the same project. Having said that, maybe it would be good to filter out duplicates that have the same name-color pair.

@davidlamhauge

This comment has been minimized.

Copy link
Contributor

davidlamhauge commented Feb 12, 2019

When I import a palette, I expect that palette to be imported. For me, it's as simple as that. The old palette should be deleted and the new imported, as with the .xml file.
What you have with the ,gpl is not an import, but a merge. The ability to merge a palette into another palette would be a nice feature, that I would like to have, but it should not be normal behavior.

@scribblemaniac

This comment has been minimized.

Copy link
Member Author

scribblemaniac commented Feb 12, 2019

@davidlamhauge When you import an image, do you expect all of the other images to be deleted? No obviously not, so why should it have a different meaning for palettes?

Edit: I would call what you are describing as opening a palette rather than importing. At least that's how I think about it.

@davidlamhauge

This comment has been minimized.

Copy link
Contributor

davidlamhauge commented Feb 12, 2019

If those are the meanings behind the words, then I would like to be able to open a palette rather than importing it, @scribblemaniac . If I later want some colors from another palette, I can import that palette into my palette. The colors I don't want can be erased, and voilá - I have a new palette.
I now have a palette, with some colors from another palette, and I want to export it... Or do I save it? For consistency it should be Open/Save or Import/Export, for equivalent operations. I hope you understand.
So what words should we use? I'll go for:
Open palette: Palette replaces existing palette.
Import palette: Palette is appended to the end of existing palette.
Save palette: Palette is saved as *.xml
Export palette: ? (the same as Save, or should one be able to export selected colors to a .xml file?)

@scribblemaniac

This comment has been minimized.

Copy link
Member Author

scribblemaniac commented Feb 12, 2019

If I later want some colors from another palette, I can import that palette into my palette. The colors I don't want can be erased, and voilá - I have a new palette.

Yes I like this workflow, but the only way this is possible is if the old palette isn't deleted.

I now have a palette, with some colors from another palette, and I want to export it... Or do I save it? For consistency it should be Open/Save or Import/Export, for equivalent operations. I hope you understand.

Your right there is is also a save/export distinction I did not consider.

Open palette: Palette replaces existing palette.
Import palette: Palette is appended to the end of existing palette.
Save palette: Palette is saved as *.xml
Export palette: ? (the same as Save, or should one be able to export selected colors to a .xml file?)

I mostly agree with the definitions here, although I would put export palette as saving to non-Pencil2D palette formats, ex. *.gpl. I don't think we need need four different actions though. I would have an action for importing, an action for saving/exporting (ie. write *.xml or *.gpl), and one action to delete all current palette colors. To emulate the open palette action, you would clear the current palette and then import the new palette.

There was some talk about having multiple palettes, which will probably happen someday. In that case the distinction between importing and opening would probably be adding to the exiting palette vs creating a new palette respectively. In that case two separate actions would be more useful, but if we're following the image import behavior, the users would instead create a new empty palette and then import the palette into it to emulate that opening behavior.

@davidlamhauge

This comment has been minimized.

Copy link
Contributor

davidlamhauge commented Feb 12, 2019

I'm not sure that I understand this:

To emulate the open palette action, you would clear the current palette and then import the new palette.

Does it mean that you Open it by Importing it in a special way? Or do I clear the palette myself, and then import?
Otherwise I agree. Of course Export would be saving in .gpl or other formats.

@scribblemaniac

This comment has been minimized.

Copy link
Member Author

scribblemaniac commented Feb 12, 2019

Does it mean that you Open it by Importing it in a special way? Or do I clear the palette myself, and then import?

The latter option. My idea is just to simplify the interface as much as possible. Clearing the palette is a useful function for other things too, and this way we avoid confusing the users with this difference between opening and importing a palette.

@Jose-Moreno

This comment has been minimized.

Copy link
Member

Jose-Moreno commented Feb 12, 2019

@davidlamhauge @scribblemaniac In large productions "Opening" a palette means a palette file was created by another department and exists to be opened in the scene file.

The user (color stylist) expects a certain color scheme to be found in that palette to correspond for different elements of said scene, e.g a character color model, a background color model, etc.
In that sense opening would qualify as "replacing" a palette. Because you only need that particular palette open.

On the other hand, If you need multiple palettes open for your different scene elements, importing a palette file means the user understands it will bring a previously created color scheme and merge that into the current "working" palette. So you create a big palette that contains all the necessary colors (adequately labeled of course) for that specific scene.

Both behaviors are required, as such we should separate them in the palette options then. Create an "open" palette action and leave the "import" palette action to be used as the merger functionality.

I also agree that repeated colors should be merged UNLESS the user chooses to, because sometimes there are colors that can have the same characteristics, but are meant to be used differently, particularly for vector workflows in toonboom harmony or similar software since if you change a vector swatch the color will change on every place where the swatch was instanced.

For example. You can have a black line color exclusively for the drawing outline, but also a black color for the fills. These would be separate entities for the color stylist and should not be merged by HUE, so in that sense maybe merging by name would be better.


As a side note I recall David had mentioned on discord about having multiple palettes open at the same time, but not merged, and while we discussed it we never formally made the feature request.

In Harmony the way the handle multiple palette files is related to how they are stored and visually they have an additional "advanced" panel where you can select multiple linked or imported palettes that are related to individual elements (drawing nodes), scenes (scene files) or environment / global (full projects) palettes interchangeably.

For inspiration on that you can read more here: https://docs.toonboom.com/help/harmony-14/paint/colour/about-palette.html

https://docs.toonboom.com/help/harmony-12/paint/Content/_CORE/_Workflow/012_Colour_Styling_Colour_Models/003_H2_Palette_Lists.html

@CandyFace

This comment has been minimized.

Copy link
Member

CandyFace commented Feb 12, 2019

I agree that open and import should be separated. The behaviour of importing should be consistent with similar actions like importing an image, sound etc... thus i'd expect it to append to the current palette. On the other hand, it sounds weird to open a palette, since you're not actually opening the palette but importing one and adding it, so it would probably be better to keep it within the import tab and improve its dialog instead.

The new dialog would contain:

  • A field to show the filepath
  • a radio button
    • merge with existing palette

If the button is not ticked, then the palette will simply append to the current palette, otherwise it will clear the current palette and add the new.

In a future iteration it would also contain a dropdown to select which palette to import to but since that's not implemented yet, there's no reason to talk about it.

Additionally I think it would also be nice to preview the palette before confirming the import.

As for removing duplicates, that's a feature that has multiple uses, thus I'd put it under the drop down menu in the palette widget for easy access.

@scribblemaniac

This comment has been minimized.

Copy link
Member Author

scribblemaniac commented Feb 21, 2019

sometimes there are colors that can have the same characteristics, but are meant to be used differently

That is why I proposed merging only if the color and name match. Presumably different uses would use different names, otherwise they would be indistinguishable to the user.

I'm still not sold that we need two functions, it seems like we're presenting the user with more options than they need. You can easily remove the old palette if it's left in, especially with a clear action, but you can't easily add it back.

I've just been reminded about the additional challenges with vector color association to the palettes. If the behavior is replace, what happens to the old linked colors? Here's the options:

  • They are all deleted. Very bad as it all the vector coloring you've done so far is basically reset.
  • The user is asked whenever there is an issue. And you thought the autosave was annoying.
  • Only unlinked colors are deleted. Now you've got inconsistent behavior with users scratching their heads at why some of the old palette won't go away, or why some of the old palette is missing.
  • All of them are left the same. Now it's just the import behavior.
  • Indexes are unchanged. This is the worst possible option, potentially leaving the vector layer in an inconsistent state or changing all of your colors. And this just so happens to be exactly how we are doing things right now. 🤦‍♂️
  • Some kind of "smart replace" algorithm is used. It might match the names for example. But this would only ever work for very specifically designed palettes.

If you haven't gathered so far, I'm still against having two explicit actions, mostly for UX reasons. If we're set on providing such functionality, the palette import window is not a bad idea though. It would probably be easier for the user to understand something like an "Override old palette" checkbox than the import vs open vs whatever else that even we struggle with defining. I also have a proposal that's been partially finished for many months for more palette formats, which may benefit from an interface where we can add new settings as necessary.

@davidlamhauge

This comment has been minimized.

Copy link
Contributor

davidlamhauge commented Feb 21, 2019

I think it's fine to call it Import Palette, and then have the option to tick a checkbox with "Override old Palette" or "Replace old Palette".

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.