Seems to work pretty well for me! Wow, I didn’t realize there would be so many other issues with switching on-the-fly, but that makes sense. The only things I notice are that mypaint:version isn’t in there; but it sounds like that’s a different topic anyway. I was thinking mypaint:version might be useful for this but I guess eotf is the only really important thing to know. Oh, I also noticed that the background layer mode is always pigment, even for legacy 1.x. But, that hardly matters (mypaint just blits it anyway), but I suppose other programs might appreciate that being Normal as well (Gimp and Krita are fine). Ah, speaking of which, we might want to dump the non-namespaced background_tile; https://github.com/mypaint/mypaint/blob/master/lib/layer/data.py#L1014-L1021. Although, I don’t know why we’d want to stop reading it for old files, as the comment suggests. . .
Yes, I think we should add such a field, but since it isn’t strictly related to the eotf compatibility I didn’t include that change in these commits.
Good catch! Since the mypaint:eotf attribute takes precedence it won’t be an issue for files edited in the 2.0 release, but old files that were edited (or just re-saved) in the 2.0-alpha releases will be interpreted as 2.x files by the check in this branch. We can just omit checking the comp-op of the background layer to avoid confusion*.
I think you’re right about that the background layer should just always use normal compositing.
* Although it’s arguable whether such files should be treated as 1.x files, given that we know they were at least edited in a 2.0-alpha. Perhaps it’s not sensible to try to make guarantees for files created/edited in unreleased versions anyway.
Yep, 2.x files will not be fully compatible with earlier versions of MyPaint in the general case anyway so we should drop it now. As for reading it, apart from the slight increase in complexity of loading .ora files, I don’t see a reason to drop support for the old attribute either. I think the comment envisioned that v2.0.0 would be released at a much later point in time.
One thing I did not think of when writing this was that the incompatibilities are such that they can actually crash MyPaint 1.2.1 under certain circumstances. Specifically, if in 1.2.1 you pick a brush from a stroke created in 2.0 with more control points in a mapping than the old maximum (8) the application will just shut down immediately due to a failed assertion in libmypaint.
There’s nothing we can do about the existing behavior in 1.2.1, but the question is if we should change the strokemap attribute and sacrifice the ability to pick any stroke from files saved in 2.x, or accept that 2.x .ora files will not be safe to use in 1.2.1 in the general case.
We could always release a new v1.3 MyPaint for this specific thing, I suppose, as that control point thing was backported on the libmypaint 1.3 branch. But I think in general we shouldn’t worry too much about old versions being able to open files created by newer versions. Maybe going forward it would be wise to only open ORA files that have the same major (mypaint) version or older. If you try to open a newer version we could present a warning perhaps, and suggest upgrading MyPaint or continue at their own risk, maybe?
Yeah, we probably shouldn’t worry about those scenarios. Hopefully the autosaving will protect against the rare cases where this actually affects users. Opening and re-saving .ora files in Gimp or Krita is a workaround for stripping the “dangerous” data if it turns into a real issue for someone.
If that’s not enough we could also take the Inkscape route and differentiate between MyPaint OpenRaster and Plain OpenRaster in the save dialog options, where the latter includes none of the MyPaint-specific data. However, I think that should wait until it’s clear whether this is a real problem or not.
And yes, we should add a compatibility check and warning (with option to never show it again) starting now (better late than never). Adding an attribute
mypaint:min-compat-version or similar means that we can have the warning only show when we know that the new files will not be fully compatible, and not just always warn for any newer version (e.g. if 2.1, 2.2 and 2.3 breaks nothing, but 2.4 does, the
min-compat-version would be 2.0 for the first three and then 2.4 until another compatibility change occurs).
Does that sound reasonable?
That all sounds very reasonable to me, but I’m wondering if this follows the spirit of semantic versioning (https://semver.org/). This gives me pause:
If even the tiniest backwards incompatible changes to the public API require a major version bump, won’t I end up at version 42.0.0 very rapidly?
This is a question of responsible development and foresight. Incompatible changes should not be introduced lightly to software that has a lot of dependent code. The cost that must be incurred to upgrade can be significant. Having to bump major versions to release incompatible changes means you’ll think through the impact of your changes, and evaluate the cost/benefit ratio involved.
Hmm. . . Maybe we just bump major versions more often instead of mypaint:min-compat-version? Any Mypaint 2.x ORA file should open up fine in any Mypaint 2.x program? Maybe we wind up like the browsers and have MyPaint 25.x within the next year or two?
There is an argument for that, and I’ve been thinking about it myself. At the end of the day, I’m a bit dubious about the use of semantic versioning for things that are neither libraries nor devtools with APIs. Can end users be expected to know about the meaning of the different numbers, and make decisions about e.g. not upgrading based on them?
My experience is that users mostly understand that a file saved in v3.0 of some program will not necessarily work in v2.0, but they do not expect the inverse to be the case as well. When something is backwards compatible, but not forwards compatible, it is the minor version field that is different (e.g. 1.1 vs 1.2), whereas only API-breaking changes require bumping the major version number (I know you know this, I’m just stating it for the benefit of the discussion).
I guess the question is what the API is in this context.
There’s also the distinction between something being incompatible and something not being fully supported. For example, files saved in 2.0 using eotf=1.0 and at most 8 control points in all brush strokes will be fully compatible with 1.2.1, but any stored views in that file will not be usable, and will be lost upon resaving. I think we should reserve changing the hypothetical min-compat-version (maybe min-supported-version is better) only when things can actually crash older versions, so there may even be that no such breaking changes are made in any of the 2.x releases.
The difference in user warnings would be something like this:
File version > App version ->
“artwork.ora” was created in MyPaint 2.3, saving it with MyPaint 2.1 can result in data being lost
File min-compat version > App version ->
“artwork.ora” requires MyPaint 2.4 or newer to function correctly in all cases. This file may be unstable in older versions. Are you sure you want to open it?
Regardless, since these kinds of checks and warnings can’t be added retroactively, I think we should add them in some way or another for 2.0.
Edit (addendum): I do agree that it would be nicer to use only the file and app version fields and not introduce additional complexity. I guess we should just make sure to not break things within major versions, warn users about incompatibilities when
app major < file major and warn about potential data loss when
app major == file major && app minor < file minor, with a checkbox/setting to disable such warnings in the future.
In other words, ignore what I wrote previously, the app and file versions should be enough. The only downside is unnecessary warnings if e.g. files saved in 2.4 are fully compatible with 2.3, because no file-affecting changes were made for that release.
One more thing I just thought of: I think that the
mypaint-settings.json file is read and re-written regardless of whether the keys and values are recognized, which means that future features (in 2.1+) making use of them should be written to handle the setting values being in an inconsistent state, due to the document being edited in and re-saved in an earlier version (e.g: a value in settings refers to a specific layer which was renamed/removed). I guess that should be the case anyway, but this would be an example of why it’s not just good practice.
Compatibility really is a a quagmire.
It really is! I see your point about preserving data from a newer version being a bad thing. You could really get mixed up if you edited a newer file with an older program. I guess we ought to just live with that and introduce the warning messages as a compromise.
After a number of… opportunities to learn more about layer internals, I have a fully working implementation of on-the-fly, reversible eotf flipping, including strokemaps, background, etc.
The only thing I really didn’t like about the implementation in the previously linked branch was that it didn’t (and still doesn’t) provide a nice way of switching between modes. That functionality should now be easy to provide, while also allowing for a nicer solution to the scratchpad problem.
However, it may take a couple of evenings to get everything structured in a nice way (within reason).
On a related note, file version warnings are added in the branch.
Hello, is this feature gets into the release finally? It seems not - I can not see the Compatibility tab, right?
I love MyPaint, but I am not getting used to Pigment (I tried), so I would like to know if there is a easy to turn off Pigment (including layer and brushes) ? Thanks!
The compatibility settings and tab are in the 2.0 beta, and will be a part of the final release (which will probably be in a couple of days). If you can’t see it in the 2.0 beta, you may need to make the setting window larger, or use the arrow buttons to navigate the tabs - it is currently the rightmost tab.
Thanks a lot for your quick response! It seems that my linux distribution has not pick up the beta release yet. I will wait. I tried AppImage for beta, I can see the Compatibility tab.