Problem of resource deduplication and aliasing in Krita
Closed, ResolvedPublic

Description

Problem of resource deduplication and aliasing in Krita

Usecase 1: Aliasing of imported resources

  1. Resource creator releases bundle_v1.bundle, which includes brush.kpp brush, which links to shine.ggr gradient.
  1. User imports the bundle, but his folder storage already has his own shine.ggr gradient with a different content. Right now Krita will fail on that.

Usecase 2: Aliasing of versioned resources

  1. Now resource creator releases bundle_v2.bundle, which includes the second version of both brush.kpp brush, which links to a new version shine.ggr gradient.
  1. The user imports the second bundle (alongside the first one). The imported version should override the older versions of the resources. It should somewhat work with the current design (see "Related problem of renaming resources on import" below)
  1. If some brushes in the user's folder storage linked to the older version of shine.ggr from the previous version of the bundle, they should still link to the older version. The current implementation somehow handles that, because the version of the resource is encoded into its name.

Problem definition

  • Some resources can link to other resources. E.g. a brush preset can link to a brush tip, a pattern and a gradient.
  • We have no standard approach for identifying the resources we link to. In most of the places we link by a 'filename', but in others we also use 'md5' and 'name'
  • when a resource has an embedded resource, we can load that into an internal storage, but it will still have to be linked as a filename, md5 or name
  • the same resource can have multiple versions, which can be linked by other resources at the same time

So, the main problem is that we have no unique identifier that would globally identify a specific resource (and a version of a specific resource). We cannot uniquely identify the resource even inside a single installation of Krita using 'resourceId', because multiple versions of the same resource will have the same 'resourceId'.

As a side note, we cannot use md5 as a unique identifier of the resource easily, because every saveToDevice() operation on the resource will change its md5 (due to inconsistencies of version and settings in libpng).

Possible solutions

  1. Consider filename as a unique identifier of the resource and its versions.
    • [pro] most of our code uses filename as a key to link to the resource already, so it should work out-of-the-box
    • [con] we need to encode the version of the resource into the filename. That is already implemented, but that might be not very pleasant-looking from the user's point of view (or is it?)
    • [con] two versions of the resource created on different machines will still alias, because they would have the same version, but different content. And this case will have to be resolved somehow.
  1. Implement "scoped resource url resolving". That is, when the user has two resources called "papergrain.pat" in two bundles, the filename will be resolved into the resource that is placed in the same bundle ("scope"), where the source rtesource is placed.
    • [pro] we have a bit more freedom in how the resources can be named. They are allowed to alias, unless they are stored in the same bundle.
    • [con] we have to significantly rewrite the resource fetching sites.
    • [con] leaf-resources will have to what bundle/scope they belong to
    • [con] there is still a usecase when this scheme breaks. If the user wants to create a brush preset in the folder storage that links to a (aliased) pattern from a bundle, he will not be able to do that. The version from the folder storage will always be preferred.
  1. Implement some form of 'uuid' for the resources. It can be stored either in the resource itself or in the bundle/storage metadata, like it happens with tags atm. ASL and ABR resources do already have such UUIDs.
    • [pro] that solves the usecase ideally
    • [con] it is difficult to store the id inside some types of resources, like .pat and .gbr. For them the ID will have to be stored somehow separately, in the bundle or storage metadata, alongside the tags.

Related problem of renaming resources on import

Right now the implementation uses the first approach, that is, filenames for the versioned resources encode the version of the resource.

Though there is a little problem right now. When a resource is imported via addResource it gets renamed to have a '.0000.' version in its filename. Which makes in undiscoverable for the resources that link to it.

I believe that the problem is in the fact that we "abuse" addResource to do things that it was not supposed to do. When we call addResource it saves a new resource from memory into the persistent storage. When performing this save operation, it does not only change/generate the name, but also changes md5-sum(!) of the resource. And we cannot avoid that md5-sum change, because it will always happen (due to different libpng version and/or settings).

To solve the problem we need to add a separate method importResource that would copy the resource into the persistent storage in byte-to-byte basis (using a QIODevice or filesystem URL, doesn't matter). This method would just forward the QIODevice or url to the storage and let it copy the data. Then both MD5 and filename will be kept the same.

And to avoid confusion in the future we could also rename addResource into something like addNewResource or createResource.

dkazakov created this task.Apr 12 2021, 8:58 AM

2: Krita wil load the existing shine.ggr, not the new one, but the new one will be in the database (and marked as inactive). The solution would be to prefer finding dependent resources in the storage of the owner resource.

3: Importing a bundle with a duplicate resource results in an inactive resource. The original resource is kept active.

4:

The imported version should override the older versions of the resources.

That is debatable. Currently the new resource is entered in the database but set to inactive. Since we first import resources from the folder storage, then bundles, the folder storage contents override the bundle contents, and that seems correct to me.

5:

The current implementation somehow handles that, because the version of the resource is encoded into its name.

That is an abuse of the versioning system, and besides, it doesn't work. We should not use version numbers in filenames to determine which dependent resource gets loaded (see 2.).

Dependent resources should be stored using the original filename, and Krita should load the latest version of the active resource with that filename. This is why filename in the resources table should be stable, and the resource loading code should use the latest version's filename from the versioned_resources table.

We have no standard approach for identifying the resources we link to. In most of the places we link by a 'filename', but in others we also use 'md5' and 'name'

And we cannot easily change that because of compatibility, unless we are happy to break that with 5.0. I'm fine with that.

when a resource has an embedded resource, we can load that into an internal storage, but it will still have to be linked as a filename, md5 or name
the same resource can have multiple versions, which can be linked by other resources at the same time

For dependent resources, we should not load the version of the resource that was used when the resource was created, but the last version. Changing a brush tip should change the presets that use that brush tip, otherwise the artist would be really confused.

We probably should consider:

  • keeping the filename in the resources table stable or add an original_filename column that can be used to find a resource by filename
  • add an original_md5 column to the resources table and use that for findbymd5

So, the main problem is that we have no unique identifier that would globally identify a specific resource (and a version of a specific resource). We cannot uniquely identify the resource even inside a single installation of Krita using 'resourceId', because multiple versions of the same resource will have the same 'resourceId'.

This is not a problem, because that is one resource, and the last version should be used.

Implement "scoped resource url resolving". That is, when the user has two resources called "papergrain.pat" in two bundles, the filename will be resolved into the resource that is placed in the same bundle ("scope"), where the source resource is placed.

Yes, that would be an improvement.

[con] there is still a usecase when this scheme breaks. If the user wants to create a brush preset in the folder storage that links to a (aliased) pattern from a bundle, he will not be able to do that. The version from the folder storage will always be preferred.

The user will never see that bundled resource on its own -- since it's marked as inactive because it duplicates a pre-existing resource with the same filename and resource type. So this cannot happen.

it is difficult to store the id inside some types of resources, like .pat and .gbr. For them the ID will have to be stored somehow separately, in the bundle or storage metadata, alongside the tags.

And that cannot work, because people don't get their pat or gbr files through Krita bundles, but through zipfiles downloaded from, for example, deviantart.

Related problem of renaming resources on import

  • resources that are imported should be copied and keep their filename
  • resources that are created for the first time should not have a 0000 in their filename, but the filename the user has decided upon

And to avoid confusion in the future we could also rename addResource into something like addNewResource or createResource.

I'd say addResource should be renamed updateResource, and we need a createResource and importResource method.

I'd say addResource should be renamed updateResource, and we need a createResource and importResource method.

We already have an updateResource method which does it better than the current addResource because it adds a new version and whatnot. addResource should be renamed to createResource, in my opinion, and a new method should be made for addResource. Congrats Dmitry for finding out this issue.

The user will never see that bundled resource on its own -- since it's marked as inactive because it duplicates a pre-existing resource with the same filename and resource type. So this cannot happen.

I don't think this solves the problem, because the user can at any moment un-inactive it in the Resource Manager. Since it's not the same resource, there is no stopping them from doing so.

Also, I think that without asking the user, we *cannot know* if the resource with the same name is the same resource or not. I'm pretty sure there is plenty of ink.kpp brush presets around. I suspect the same goes for canvas.pat or watercolor.pat that could be used as a texture in some brush presets. There might be some name-clashing even in less obvious examples. However asking the user about every resource sounds a bit tedious...

I thought about name-scoping and I'm no longer supporting this idea because of the possibility of the user using a resource from one storage for a resource in another storage.

Changing a brush tip should change the presets that use that brush tip, otherwise the artist would be really confused.

The artist might also be confused because the preset they used a half a year ago no longer works the way they expected. But I agree that this would be more consistent, I guess.


At some point I entertained the idea of giving up the idea of storages inside the resource folder at all - in that case, bundles would be just for importing an exporting the resource (for which Krita would have complete control over), but it would require implementing saving of one layer style and has a big potential of blowing in our faces at some point in some way, because it wasn't really thought through yet.

Another probably-bad-idea would be to get rid of dependant resources, but that has even more known issues: - requires implementing new version of kpp and whatnot, - bigger kpp files because the resources are not reusable anymore.

We already have an updateResource method which does it better than the current addResource because it adds a new version and whatnot. addResource should be renamed to createResource, in my opinion, and a new method should be made for addResource. Congrats Dmitry for finding out this issue.

No, there is only KisResourceStorage::addResource . And it really is not suitable for creating or importing new resources as is because it changes the filename immediately.

I don't think this solves the problem, because the user can at any moment un-inactive it in the Resource Manager. Since it's not the same resource, there is no stopping them from doing so.

No, but that is their action. The consequences are not our problem.

Also, I think that without asking the user, we *cannot know* if the resource with the same name is the same resource or not. I'm pretty sure there is plenty of ink.kpp brush presets around. I suspect the same goes for canvas.pat or watercolor.pat that could be used as a texture in some brush presets. There might be some name-clashing even in less obvious examples. However asking the user about every resource sounds a bit tedious...

No, we cannot know that. That's why we have a resource manager, so the user can select the one they want -- but by default the first resource for a certain type imported into the database with a given filename is the one the user sees.

I thought about name-scoping and I'm no longer supporting this idea because of the possibility of the user using a resource from one storage for a resource in another storage.

On the other hand, in the much more likely case of a user importing a bundle with a perset that uses a bundled tip with a duplicate filename, this preset should load that tip from the bundle storage.

I guess that what's really biting us here is that we do not map this relationship in the database and in the bundle to begin with.

The artist might also be confused because the preset they used a half a year ago no longer works the way they expected. But I agree that this would be more consistent, I guess.

Given that it's the artist who has made a new version of the brush tip, they shouldn't be confused. I foresee much more users calling out "hey, I changed the brush tip, why isn't my preset updated" than the other way around.

At some point I entertained the idea of giving up the idea of storages inside the resource folder at all - in that case, bundles would be just for importing an exporting the resource (for which Krita would have complete control over), but it would require implementing saving of one layer style and has a big potential of blowing in our faces at some point in some way, because it wasn't really thought through yet.

Another probably-bad-idea would be to get rid of dependant resources, but that has even more known issues: - requires implementing new version of kpp and whatnot, - bigger kpp files because the resources are not reusable anymore.

No, neither of these approaches is something I would want to do.

I guess that what's really biting us here is that we do not map this relationship in the database and in the bundle to begin with.

That would be quite useful even for GUI and in some non-essential other places. If I'm not mistaken, Dmitry said however that it would require high maintaince.
I'm not sure if it solves our issue, but I hope it could, then the maintaince would be probably worth it.

No, there is only KisResourceStorage::addResource . And it really is not suitable for creating or importing new resources as is because it changes the filename immediately.

Ahh, I was thinking of the KisResourceModel::updateResource.

rempt added a comment.Apr 12 2021, 1:18 PM

That would be quite useful even for GUI and in some non-essential other places. If I'm not mistaken, Dmitry said however that it would require high maintaince.
I'm not sure if it solves our issue, but I hope it could, then the maintaince would be probably worth it.

Since we have a database, it should reflect the relationships between our resources.

Ahh, I was thinking of the KisResourceModel::updateResource.

That ultimately wends up in KisResourceStorage through KisResourceLocator.

2: Krita wil load the existing shine.ggr, not the new one, but the new one will be in the database (and marked as inactive).

Right now it will make brush.kpp invalid, because it links to a different resource.

The solution would be to prefer finding dependent resources in the storage of the owner resource.

That is what I named "scoped" loading, which also has its own problems.

3: Importing a bundle with a duplicate resource results in an inactive resource. The original resource is kept active.

That is not a "duplicate resource", it is a new version of the resource. The resources that link to old and new version of it should still be valid after such importing.

Dependent resources should be stored using the original filename, and Krita should load the latest version of the active resource with that filename. This is why filename in the resources table should be stable, and the resource loading code should use the latest version's filename from the versioned_resources table.

Can you try to forget about the implementation detail for a minute and think about the problem itself for a bit of time?

The problem is, we should guarantee a simple invariant: the resource must link to exactly the same resource (and the same version of it) that was planned by the author of the resource.

There is no much difference how we implement that. And, speaking truly, I don't think the user actually cares whether we abuse filename for versioning or not. It needs the brushes to work in the same way on all the devices he loads them on, that is the main requirement. Otherwise, @Deevad (or we) will have a lot of bug reports telling that @Deevad's bundle "doesn't work".

Also, don't forget that the user has no way to select a filename for brush presets in Krita. I'm not really sure he can actually find it via GUI. The user has "name" of the resource, that is what is displayed to the user and this one is kept stable on version update. Why we should care about filenames cleanliness then?

We have no standard approach for identifying the resources we link to. In most of the places we link by a 'filename', but in others we also use 'md5' and 'name'

And we cannot easily change that because of compatibility, unless we are happy to break that with 5.0. I'm fine with that.

We can do an easy transition by abusing filenames for some time (and implement some form of unique ids some time later).

For dependent resources, we should not load the version of the resource that was used when the resource was created, but the last version. Changing a brush tip should change the presets that use that brush tip, otherwise the artist would be really confused.

That will cause a lot of bugreports to us and @Deevad telling that his bundles "don't work".

I guess that what's really biting us here is that we do not map this relationship in the database and in the bundle to begin with.

You are abusing the idea of the database now. Don't forget that initially, the database was planned only as a caching utility. We shouldn't abuse it by storing information than not actually needed for caching. Even tags were originally planned to be stored in a persistent storage in desktop files.

You are abusing the idea of the database now.

No, I am not.

Don't forget that initially, the database was planned only as a caching utility. We shouldn't abuse it by storing information than not actually needed for caching.

We use the database to provide a view of the resource and tagging system. The relation between resources is part of that view.

Even tags were originally planned to be stored in a persistent storage in desktop files.

Which is irrelevant in this discussion, and besides, the whole system to read and write tags from desktop files has been implemented. We just don't save user tags at the moment, but everything is ready for that, should we think it's a priority.

rempt closed this task as Wontfix.Apr 15 2021, 11:05 AM
rempt claimed this task.

Can you try to forget about the implementation detail for a minute and think about the problem itself for a bit of time?

I don't think this discussion is making much sense. You're fixated on one issue -- bundles that replace existing bundles with a new version of a resource, which isn't the whole problem at all. And nothing I say seems to get through, so let's close this task.

dkazakov reopened this task as Open.EditedApr 22 2021, 11:40 AM

Can you try to forget about the implementation detail for a minute and think about the problem itself for a bit of time?

I don't think this discussion is making much sense. You're fixated on one issue -- bundles that replace existing bundles with a new version of a resource, which isn't the whole problem at all. And nothing I say seems to get through, so let's close this task.

I'm not fixated on just "bundles replacing bundles". I point out a real problem. You expect all resource filenames in the world be unique, but that just cannot work in real life.

You also break the main expectation that the user has from "file format": the file (in our case resource) must look exactly the same on all the systems that open it. With your "fixation" on filenames the look of the resources when they are loaded on the user's system is just "undefined behavior". They may start look differently from how the author expected them to look because of the numerous reasons the user doesn't control and has no idea about. For example, some other bundle may have a different resource with the same filename (like watercolor.png) or the order with which different versions of the same bundles are added will define how the resource will look. The user has no idea about such implementation details (and shouldn't have).

UPD:
And the main reason why I try to raise this problem now, before we release Krita 5.0, because you want to bake this incorrect behavior into the file format and into the database scheme. After we release Krita 5 and start getting all these bugs like "I published brush bundle and it works wrongly on the user's machine and I have no idea why", it will be extremely difficult to fix that.

I am starting to get the feeling it's better to talk to artists about this. It's too focused on this edge case of everyone naming their unique resources with the exact same string.

I am starting to get the feeling it's better to talk to artists about this. It's too focused on this edge case of everyone naming their unique resources with the exact same string.

It is also related to how we name/manage versioned resources. The artists don't have access to the choice of names of these resources, that is done internally.

rempt added a comment.Apr 22 2021, 1:24 PM

Can you try to forget about the implementation detail for a minute and think about the problem itself for a bit of time?

I don't think this discussion is making much sense. You're fixated on one issue -- bundles that replace existing bundles with a new version of a resource, which isn't the whole problem at all. And nothing I say seems to get through, so let's close this task.

I'm not fixated on just "bundles replacing bundles". I point out a real problem. You expect all resource filenames in the world be unique, but that just cannot work in real life.

You also break the main expectation that the user has from "file format": the file (in our case resource) must look exactly the same on all the systems that open it. With your "fixation" on filenames the look of the resources when they are loaded on the user's system is just "undefined behavior". They may start look differently from how the author expected them to look because of the numerous reasons the user doesn't control and has no idea about. For example, some other bundle may have a different resource with the same filename (like watercolor.png) or the order with which different versions of the same bundles are added will define how the resource will look. The user has no idea about such implementation details (and shouldn't have).

UPD:
And the main reason why I try to raise this problem now, before we release Krita 5.0, because you want to bake this incorrect behavior into the file format and into the database scheme. After we release Krita 5 and start getting all these bugs like "I published brush bundle and it works wrongly on the user's machine and I have no idea why", it will be extremely difficult to fix that.

Like I said, nothing I'm saying is getting through. You're not trying to understand the problem at all, you seem more fixated on proving that you had the right idea when you implemented resource versioning. And that implementation HAS problems. It's fine if you want to keep this task "open", but I am going to solve the problems I see. When I next get some time to do some actual coding...

Hi, @rempt!

I'm sorry, but that is you who refuses to listen, not me. I intentionally avoid mentioning "my implementation" or "your implementation" in this thread. I don't really care how it is going to be implemented at all. The thing I do care about is the wrong requirement you declared during the discussion, which will break user's workflow and provides us with tons of bugs that will be very hard to fix.

Could you listen at least once and comment on technical side of the problem instead to getting down to personalities?

The problem is in this requirement that you declared:

The imported resource overrides the existing resource with the same filename and disables it.

I believe that this requirement is inherently wrong, because mere bundle importing will change the look of the existing user's resources that are not related to that bundle (because they may link to the overridden resource).

I insist that this requirement should be changed into the following:

The imported resource or bundle should look exactly the same as when the author created it.  It should not depend on the set of existing resources the user has, neither should it change any existing resource (except when overriding it completely).

If you agree to change the requirement, we can discuss the details of the implementation. Encoding resource version in the filename is only one (and not ideal) solution to the problem. We can do anything else, given it satisfies the requirement.

As I have said a couple of times: resource versioning has NOTHING to do with resource de-duplication. Any discussion that confuses the two topics is useless.

dkazakov added a comment.EditedApr 23 2021, 10:42 AM

Can you comment on the requirement instead? There is no reason to discuss whether "resource versioning" has anything to do with "deduplication" before we agree on the basic requirement to the system.

UPD:
Btw, if you reread the title of the task, you'll see no mentioning of the versioned resources. They are only a small particular case of a more general problem.

tymond added a comment.May 3 2021, 8:38 PM

I was thinking about this issue. It's a bit complex; I agree with Dmitry there is an issue, I disagree that putting a version number in the filename would solve it completely though. I don't think we can solve it, really, at least without user input, which might've been a bit annoying... so I'd suggest just trying to make sure *it doesn't happen*. It's not trivial, but might be easier and more stable than other solutions. (Except for UUID, which we cannot pursue at this point).

  • The most vulnerable resources for this issue are all resources that can be used as dependant resources. I think for now those are brush tips, patterns and gradients. For now we don't have any versioning implemented for patterns of brush tips, only for gradients.
  • But resources with the same name can be the same resource, so should be a subject of deduplication.
  • Deduplication is necessary for dependant resources (since now Krita adds all dependant resources to the bundle). It's not necessary, but good-to-have for other resources. So we need to find a good way. (Note: we should load the resources and actually compare them, not use md5).
  • We control the default resources, since they're in the repo. I don't think there are any that have the same name (maybe except for duplicated patterns, which we should resolve before release).
  • We can control the resources as they are imported via Krita. If the user just puts stuff in resource folder... I guess we will detect it on the next opening of Krita, so we could, potentially, do it then too.
  • We *cannot* control what the user does in the resource folder. There is no way. I don't think we should even try, except for making sure that the database is consistent and that we detect new resources. (This might be a tricky point for my solution).

Hence, my suggestion is to just ensure that there are no resources with the same name, period. The appropriate time to ensure it is (1) on import, (2) on initial adding resources that are in the resource folder to the database (this includes when the user added something to the resource folder).

  1. when the user imports/adds a new resource (separately or in a bundle), check if there is a resource with the same name/filename already. If there is, check if they are identical (not based on md5, but load and compare). If they are, deduplicate (I'm not sure what to do here technically, but do something so the user doesn't see the resource - can be simple import and mark as inactive). If they aren't, change the name and filename of the resource to include "(n)" where n is the first unoccupied number (I talked with David, I believe, and they suggested (1) as a way to distinguish duplicate bundle, and I think it would fit here, too; I also believe that while .0001. or _0001 is a very version-looking number, (1) looks like a duplicate/copy, so that would fit).
  1. If there are any resources that depend on that resource, they need to be changed to include that new name/filename instead of the old one.
  1. Don't allow the user to create a resource with the same name as existing ones.

Yes, that would change the imported bundle (since we're changing the filenames and content of the resources inside the bundle). It doesn't sound ideal, but we do it already in case of .asl files (another storage), and it's one-time only, so I don't think it would cause many issues (I don't think many people care whether the bundle is byte-equal to what they've downloaded?). Later Krita can follow the folder-based versioning approach.


Problem with Dmitry's proposed approach: it's good for versioned the same resource... but not just a different resource.

  • We have person A and person B. Both A and B create a resource with the same name, and then change it twice to get version 2.
  • Person A creates a bundle at this point.
  • Both A and B change the resource several times more, so they end up with version 5.
  • Now both person A and B import the bundle.
  • If the resource was a stand-alone resource (a resource no other resource depends on), like brush preset, then... we can ignore the version information anyway: we cannot have two versions of a resource active at the same time, which means it needs to be treated as a different resource, unless it's identical with the current version of the resource of the same name (?).
  • If the resource was a dependant resource (a resource other resources depends on), then, potentially, we could benefit from the versioning information... but I'm not sure. In the case of person B, it will be a different resource altogether, why would it be considered the same? (So we need to check for indentical-ness). But even in case of person A, which means that the resource is equal to a previous version of the resource, we have an option to just choose the same resource from the Local Resources that is in the higher version, but that would break Tiar's Basic Principles #1, please see below. We could try to make sure the brush preset knows it is supposed to find the 2nd version of the resource and try to find that. But we don't have a way to ensure that the brush preset knows which version of the resource it is using (which is a more general problem altogether, but we can assume that if the user has already in their resource folder multiple resources depending on one resource, then they can in theory expect that if they make a change to the underlying resource, all the main resources will change accordingly as well; however while importing, the user is under the assumption they will get resources the same way the author made them).

But even in that case, wouldn't it be just as acceptable to just import that dependant resource as a new resource so it can show up in resource choosers and whatnot? Dependant resources are resources on their own, too, so I guess they should fall into the category from the previous paragraph as well.

So while Dmitry's proposed solution sounds sensible, I don't think it solves the problem. While my solution is a bit of a band-aid, I'll admit, I believe the proposed solution here is a band-aid but even less useful (a leaf, maybe ;) ).

I'll be waiting to hear the new Dmitry's solution that he announced he figured out but haven't written down here yet. I really hope it's better than mine, frankly, even though I think mine is doable, it's still far from ideal.


Tiar's Basic Principles (in the order of importance):

  1. Under the condition that the user manages all of their resources in Krita, their brush presets and other resources should work the same way after importing other resources.
  1. If they import a different resource (even named the same way), they should see it in the resource chooser.
  1. (Least important) identical resources from different storages should be considered the same resource and only the resource from Local Resources should be shown (Local Resources should be considered the primary source).

Much Less Important Things:

  1. Resources to be named prettily (they need to be shown prettily in krita, not in the folder, because we want most people to use Krita to manage resources, I guess?).
  1. Bundle to be untouchable/uneditable.
  1. Possibly more to follow, but I don't remember right now.

I do look forward to see Halla's take on the main rules our solution must follow.

For the principles, I would like to take that fraction of the discussion and do ask artists what they think about it and what *they* see as most important, just to ensure we do keep the goal of making their life easier in mind.


I guess that what's really biting us here is that we do not map this relationship in the database and in the bundle to begin with.

That would be quite useful even for GUI and in some non-essential other places. If I'm not mistaken, Dmitry said however that it would require high maintaince. I do hope the benefits will be greater than the cost of maintaince.
I'm not sure it would solve our issue, I'd need to think more about it, also about what Dmitry said that we're abusing database for other things. I think if we don't use my solution described here, it would mean that the function that reads the resource folder and adds new stuff to database would have to be smart about which resource depends on which. I guess it would be doable. It could also prevent the issue that my solution doesn't solve, which is when the user create a new version of a dependant resource and the main resources (those that depend on it) changes accordingly. I'm not sure if I would like this solution as a solution for this issue, but it does seem like a viable suggestion.

No, there is only KisResourceStorage::addResource . And it really is not suitable for creating or importing new resources as is because it changes the filename immediately.

Ahh, I was thinking of the KisResourceModel::updateResource. (I've seen you solved this issue already).


I hope you don't mind a long message. This issue does worry me, and when I worry, I write more ;)

rempt added a comment.EditedMay 4 2021, 8:09 AM

I have a feeling people think we deduplicate resources by filename. We don't: we use the md5sum for that.

Also, that it is possible to have two different versions of a particular resource loaded at the same time: that is not possible.

dkazakov added a comment.EditedMay 4 2021, 8:32 AM

Hi, @tymond!

Firstly, thanks a lot for joining this thread!

I disagree that putting a version number in the filename would solve it completely though

Just to make it clear. I don't tell that encoding versions in the filenames will solve all our problems. It solves a small subset of most visible problems and lets us transition to a better solution smoothly. It also adds a bit of redundancy to the system, that is, if the database is currupted (ant it will surely happen during the first several releases of Krita 5) the versioning information can be restored. But we still need a proper system for the deduplication and antialiasing.

Anyway, there is not much use in talking about the specific solutions now, when we still have no agreement about the high-level requirements to the system. Halla thinks that we have a right to modify imported user's bundles when we have a "better" version of a dependant resource with the same name. I believe that all the imported bundles should have exactly the same look as when the author created it.

If we get an agreement with the basic requirements, we can start discussing the implementation details.

Notes on your proposal

If there are any resources that depend on that resource, they need to be changed to include that new name/filename instead of the old one.

At the moment, we have no means to modify links in resources from the outside. It will be rather hard to do. Right now the resource is a blackbox, which is responsible to load everything itself. We'll have to refactor it and transfer responsibility to some other entity.

Problem with Dmitry's proposed approach: it's good for versioned the same resource... but not just a different resource.

Yes, that is a valid problem. And that is why I tell that this task is not about "writing version in the filename". The problem is much broader.

Tiar's Basic Principles (in the order of importance):

As far as I can see your principles are just a more detailed version of what I proposed above in a form of a requirement :) I'm fully agree with them.

The problem is that the porposal pushed by Halla is the opposite of it. She insists that the resources with the same name should override each other.

I'll be waiting to hear the new Dmitry's solution that he announced he figured out but haven't written down here yet. I really hope it's better than mine, frankly, even though I think mine is doable, it's still far from ideal.

Well, my current idea is to extend the idea of existing classes KisEmbeddedPatternManager and KisLinkedResourceWrapper. When releasing Kirta 5 we can change resources file format a bit and start saving the triplet of md5, filename and name. Therefore when requesting the resource, we could address it via this triplet, which would solve at least some troubles (given the limitations and fragility of md5).

Though it will not solve a lot of other troubles. E.g. what happens if we create a new version of a resource, which is linked by another resource? What if we import a resource that links the older version of an existing resource? As far as I can tell, the current layout of the database cannot handle these cases, so they should be discussed at least.

I could have tried to start that triplet-addressing refactoring, but I'm blocked but the requirements part.

I have a feeling people think we deduplicate resources by filename. We don't: we use the md5sum for that.

Deduplication on loading is one thing. Addressing the linked resource is another thing. Don't mix them.

Also, that it is possible to have two different versions of a particular resource loaded at the same time: that is not possible.

And that is a problem. It prevents addressing by MD5.

rempt added a comment.EditedMay 4 2021, 9:46 AM

Okay, let's start with this, and I'll repeat it a couple of times:

A version is not a resource, a resource has versions.

Deduplication on loading is one thing. Addressing the linked resource is another thing. Don't mix them.

Resources are added to the database, even if they have the same md5 if they have a different storage, but set to inactive. Code can access inactive resources, but, of course, if two resources have the same md5 they are the same thing.

And that is a problem. It prevents addressing by MD5.

No, that's not a problem. The md5sum is supposed to be the unique identifier for a resource. A resource should be identifiable by the original md5sum of the resource before the user started modifying it.

A version is not a resource: a resource has versions, and the last version of the resource is the one that should get loaded, until the user goes back in the version history. That is the only purposed of resource versioning. There has never been another purpose, and trying to give it that complicates things enormously.

resources file format a bit and start saving the triplet of md5, filename and name.

There is no such thing as a resources file format.

E.g. what happens if we create a new version of a resource, which is linked by another resource?

The the other resource should use the new version.

What if we import a resource that links the older version of an existing resource?

Resources link to resources, not resource versions, because versions are not resources. What you mean is "what if a resource was imported that linked to a default resource that was modified by the user" -- and the answer is clear: it should use the version as modified by the user.

As far as I can tell, the current layout of the database cannot handle these cases, so they should be discussed at least.

As you can see, once you realize that a resource has versions, but a version is not a resource, these questions are solved.

As for bundles...

Given that storages are defined by their path, and not by metadata, and given that bundle creators "usually" name their bundles bla_bla_v1.bundle, bla_bla_v2.bundle: these bundles are actually different storages, not different versions of the same storage. If the user has modified a resource from the v1 bundle, the v2 bundle will not see those modification.

If a bundle is updated, but the filename has not been changed, and both the user has modified some resources that are in the bundle, and the bundle has modified a resource, then the resource in the bundle will be seen as a new, different resource (it has a different starting md5sum). There will now be two resources with the same name, filename but different md5sum. That might confuse the user, but it is not a killing problem.

If a resource in a bundle uses a resource from another storage, and the user has modified that resource, the resource will use the modified version, even if the bundle creator had intended that the original version would be used. Since it was the user themselves who had modified the resource (which probably will happen a lot less often than saving the modifier resources under a new name), it is their responsibility, not our problem.

The real problem I ran into when we began this endless discussion is that the resource table got updated when a new version was created -- and that was wrong.

I have already started fixing that, though. As tiar originally suggested, we now have different code for importing a resource, creating a resource and saving a modified resource.

My next steps are:

  • make sure the original filename and original md5sum are in the resources table
  • make sure KoResource returns the md5sum that is stored in the database
  • find all places where we try to get a resource by name or filename, and add code to first get it by md5sum, with name or filename as a compatibility fallback
  • and then go back to importing and creating resources, which had gotten broken.

A version is not a resource, a resource has versions.

That is one of the problems of your approach, which leads to undefined behavior in numerous cases.

And that is a problem. It prevents addressing by MD5.

No, that's not a problem. The md5sum is supposed to be the unique identifier for a resource. A resource should be identifiable by the original md5sum of the resource before the user started modifying it.

You contradict yourself. md5sun is an identifier to the "resource version", not to the "resource"

Consider this example. The brush preset links to a brush/texture using MD5. This MD5 links to a previous version of the resource. When painting, the brush/texture is fetched correctlly. But when the user opens brush editor, brush/texture selection widget cannot find this resource (because it is overridden) and selects a random resource (either fallback resource or the latest version of it, depending on the cleverness of the system). It means that the look of the brush will be changed by mere opening of the brush editor.

I don't even speak about the fact that is breaks your own requirement of "use the latest version of the resource all the time".

Resources link to resources, not resource versions, because versions are not resources.

You contradict yourself again. MD5 sum identifies a resource version, not the resource.

What you mean is "what if a resource was imported that linked to a default resource that was modified by the user" -- and the answer is clear: it should use the version as modified by the user.

That is a bug that will be reported soon after we release that. It means that the look of resources in the bundle will depend on the state of the user's resources folder. That is, if the user downloaded some custom file 01_canvas.png the imported bundles will look buggy for him. And we will have a lot of reports like "why Ramon's brushes don't look on my PC like they look in the video?".

If the user has modified a resource from the v1 bundle, the v2 bundle will not see those modification.

We don't have resource-scopes at the moment. Resource doesn't know what bundle it belongs to. That is, in your example modifications to bundle_v1 will be visible in bundle_v2.

The real problem I ran into when we began this endless discussion is that the resource table got updated when a new version was created -- and that was wrong.

But this table should be updated anyway. You need to upload the new thumbnail (and, probably, some other metadata)

make sure the original filename and original md5sum are in the resources table

Are you going to store md5sums for every resource version of the resource in the resources table?

Again, I believe that the idea of "use always the latest version of the resource" is inherently wrong. It will break the workflow of both, users and resource bundle creators.

rempt added a comment.May 4 2021, 11:11 AM

That is one of the problems of your approach, which leads to undefined behavior in numerous cases.

Only if you don't understand the design.

You contradict yourself. md5sun is an identifier to the "resource version", not to the "resource"

The md5sum that's the unique identifier is the original md5sum as it was when the resource has been imported. There is no contradiction. We store md5sums for each version in the versioned_resources table.

Consider this example. The brush preset links to a brush/texture using MD5. This MD5 links to a previous version of the resource.

No, it doesn't. It should use the original md5sum to link to the resource. That's the identifier. Not the md5sum for the last version of the resource.

I don't even speak about the fact that is breaks your own requirement of "use the latest version of the resource all the time".

It should use the original md5sum to link to the resource. That's the identifier. Not the md5sum for the last version of the resource.

Resources link to resources, not resource versions, because versions are not resources.

You contradict yourself again. MD5 sum identifies a resource version, not the resource.

It should use the original md5sum to link to the resource. That's the identifier. Not the md5sum for the last version of the resource.

That is a bug that will be reported soon after we release that. It means that the look of resources in the bundle will depend on the state of the user's resources folder. That is, if the user downloaded some custom file 01_canvas.png the imported bundles will look buggy for him. And we will have a lot of reports like "why Ramon's brushes don't look on my PC like they look in the video?".

If 01_canvas.png has the same md5sum as the bundled 01_canvas.png it IS the same thing. No problem. If it isn't the same, also no problem, because it is a different resource, despite having the same filename.

If the user has modified a resource from the v1 bundle, the v2 bundle will not see those modification.

We don't have resource-scopes at the moment. Resource doesn't know what bundle it belongs to. That is, in your example modifications to bundle_v1 will be visible in bundle_v2.

The real problem I ran into when we began this endless discussion is that the resource table got updated when a new version was created -- and that was wrong.

But this table should be updated anyway. You need to upload the new thumbnail (and, probably, some other metadata)

The data we use to find the resource (filename, name, md5sum) shouldn't be updated.

make sure the original filename and original md5sum are in the resources table

Are you going to store md5sums for every resource version of the resource in the resources table?

Of course not because a version is not a resource. You keep thinking about linking to some specific version of a resource -- and that is wrong.

Again, I believe that the idea of "use always the latest version of the resource" is inherently wrong. It will break the workflow of both, users and resource bundle creators.

You are wrong. You're trying to make the versioning system, which is ONLY designed so the user can go back to earlier versions of a resource do something it should not do.

The requirement to use the user's modified version is right there in the middle of https://phabricator.kde.org/T379.

Only if you don't understand the design.

Please be constructive. I don't let myself get down to personal insults.

The md5sum that's the unique identifier is the original md5sum as it was when the resource has been imported. There is no contradiction. We store md5sums for each version in the versioned_resources table.

It means that you are trying to reinvent the "unique identifier" idea. Which breaks on the fact that we cannot save it into resources or bundles. After saving the latest version of a resource into a bundle, its "original md5" will change. Which will break all the resources that link to that and you will have to fall back to the filename, which is prone to aliasing.

Again, I believe that the idea of "use always the latest version of the resource" is inherently wrong. It will break the workflow of both, users and resource bundle creators.

You are wrong. You're trying to make the versioning system, which is ONLY designed so the user can go back to earlier versions of a resource do something it should not do.

The only thing I want to make sure is that bundles created by the bundle creator have exactly the same look on every system. And you are trying to break that.

The requirement to use the user's modified version is right there in the middle of https://phabricator.kde.org/T379.

It has nothing about the linked resources. There are only requirements for modification of "resources" as a whole.

rempt added a comment.May 4 2021, 2:37 PM

I am not trying to "break" anything, I'm trying to keep a relatively simple and sensible design working, instead of overcomplicating things. The version system simply is not suitable for what you're trying to do.

After saving the latest version of a resource into a bundle, its "original md5" will change.

At least with bundles, we can save the "original" md5sum in the xml and use that for resource finding.

I am not trying to "break" anything, I'm trying to keep a relatively simple and sensible design working, instead of overcomplicating things. The version system simply is not suitable for what you're trying to do.

This "simple" system just doesn't solve the user's problem.

After saving the latest version of a resource into a bundle, its "original md5" will change.

At least with bundles, we can save the "original" md5sum in the xml and use that for resource finding.

If you are going to implement the "unique-id" thing anyway (even though you have refused that several times previously), why can't you just add unique-resource-id and unique-resource-version-id to that XML file? That would solve all our problems. The linked resources would address the dependencies as unique-resource-version-id and use unique-resource-id as a fallback. The main cause of this dispute is that initially you refused to use "unique-id" approach and said that we should keep the bundle format unchanged. Now if you are going to change the bundle format anyway, why should we limit ourselves to workarounds that do not solve the user's problem?

tymond added a comment.EditedMay 4 2021, 8:57 PM

@rempt Do you want this behaviour for both embedded and linked resources? From the user point of view, there is hardly a difference there, and I've seen that embedded resources can be loaded from the global resource interface to save the memory, so I guess you'd want it for both.

What about documents?

What if someone makes a document with a Fill Layer with SeExpr script - which can have versions in Krita by default - and save it and then leave it for 3 years, in the meantime making another version of the script, then they open the document and it looks different, because they have another version of the script in the resource folder? It was three years, they could long time ago forgot they made another version, but they would still expect the document to be shown exactly the way it was before.

Gradients are used for various filters (and FIlter Layers are a thing), and gradients are generally, in my opinion, quite a throw-away kind of resources, I wouldn't be surprised when someone just edits their custom gradients all the time to get the specific result at a moment. Why would their previously saved file had a different projection just because of that?

Leaving the files alone, there are layer styles with gradients as a required/dependant resource, if you download a bundle from somewhere with this awesome shining gold effect, you don't expect it to suddenly end up blue in your file because you overwrote some default gradient two years ago.

And that's only because we don't have any versioning for brush tips... yet. (EDIT: Note the bug report Wolthera made recently: https://bugs.kde.org/436633 - so I guess the end point would be allowing brush tips to be versioned as well...).

I really think we need to ask artists about this. Don't need to be any technical details, just, if your solution is acceptable (with careful explanation, considering the issues it introduces and various workarounds). I can volunteer to prepare some kind of Abstract of this discussion, if needed.

And I think we do need to figure out a way to ensure imported and opened stuff works the way they worked when they were created. It's just not reliable otherwise, in my opinion.

You can say that a different version of the resource is the same resource... but there is a reason for it to be a *different version*, and that reason is that it's different and works differently, and if you download a bundle, let's say Ramon's bundle, or any other resource, you expect it to work just like advertised, and not like it ends up working based on your specific situation in the resource folder (that is, if you ever overwrote a resource that some resource in the bundle depends on).

I actually think that deduplication is the least important issue here - as in, we don't even need to do it. Sure, it would be best if we had a great method to do it, but if we can't find that, it might be just enough to ensure that (1) we don't have duplicated resources by default, and (2) there is an easy way to mark the imported duplicated resources as inactive (the new resource manager would be enough, since you can just select the appropriate new storage and mark it all inactive). Then the user would be responsible for removing any unwanted duplicates. But if we do deduplicate, we should be careful to not be too eager, I think.


FYI, that's what I gathered for now in terms of resources vs linking and embedding vs versioning. The biggest issue is when we have a resource that can be linked or embedded and which is versioned, at least n regards to the specific issue I was talking about in this comment.

Typecan be linkedcan be embeddedlinks toembedsis versnioning available from Krita GUI
brush presets--brushpatternyes
brush tipsyes----
gamut masks-----
gradientsyesyes--yes
layer styles---gradients, patterns*-
palettesyes---yes
patternsyesyes---
seexpr scripts----yes
sessions-----
symbols-----
tasksets-----
windowlayouts-----
workspaces-----
...others (documents etc.)--patterns (filter conf.), gradient (old gradient map conf.)gradient (new filter conf.), seexpr script (Fill Layer)-

*) layer styles have patterns and gradients implemented differently...

If you find any mistake here, please let me know, it was just from skimming through the source files and from my memory.

rempt added a comment.May 6 2021, 12:31 PM

I am not trying to "break" anything, I'm trying to keep a relatively simple and sensible design working, instead of overcomplicating things. The version system simply is not suitable for what you're trying to do.

This "simple" system just doesn't solve the user's problem.

And complicated system isn't fit for purpose. It's the wrong tool for this problem. I've already said a couple of times that I want to end this discussion, and just start working.

After saving the latest version of a resource into a bundle, its "original md5" will change.

At least with bundles, we can save the "original" md5sum in the xml and use that for resource finding.

If you are going to implement the "unique-id" thing anyway (even though you have refused that several times previously), why can't you just add unique-resource-id and unique-resource-version-id to that XML file? That would solve all our problems. The linked resources would address the dependencies as unique-resource-version-id and use unique-resource-id as a fallback. The main cause of this dispute is that initially you refused to use "unique-id" approach and said that we should keep the bundle format unchanged. Now if you are going to change the bundle format anyway, why should we limit ourselves to workarounds that do not solve the user's problem?

You know pretty well what the problem with the unique id is: resources outside bundles cannot have them.

... and just start working.

And just waste your time, because this code will have to be thrown away in the end, as breaking user's workflow.

If you are going to implement the "unique-id" thing anyway (even though you have refused that several times previously), why can't you just add unique-resource-id and unique-resource-version-id to that XML file? That would solve all our problems. The linked resources would address the dependencies as unique-resource-version-id and use unique-resource-id as a fallback. The main cause of this dispute is that initially you refused to use "unique-id" approach and said that we should keep the bundle format unchanged. Now if you are going to change the bundle format anyway, why should we limit ourselves to workarounds that do not solve the user's problem?

You know pretty well what the problem with the unique id is: resources outside bundles cannot have them.

I know pretty well that this problem is easily solvable: just declare that unique-resource-version-id is MD5 sum of the resource and unique-resource-id is MD5 sum of the first version of the resource. You do already plan to implement the second half of this design.

Hi, @rempt!

We still need to come to some agreement on this topic. After the Friday's discussion I tried to merge both, your feature of "dynamically linked resources" and what painters expect (based on input from @kamathraghavendra), into a set of requirements:

Requirements:

  1. Resources embedded in the bundle should always look exactly the same way as when the author of the bundle created them (given that all the dependent resources are embedded into the bundle as well).
  2. Importing a bundle should not change any resources in the resource folder (including already imported bundles), even when they have the same name, filename (and, optionally, md5).
  3. Overwriting a resource in the resource folder should warn the user that some other resources depend on it. The user may select to save the resource under a different name or choose to override all the dependent resources.
  4. If the user chose to override all the dependent resources and any of them embeds this resource, a new version of this resource should be created with updated embedded data. That is, if the user moves this resource to another PC it won't change its look magically.
  5. Basically, given a resource embeds something, this something should not change how it looks when the parent resource is transferred from one PC to another.
  6. When importing a bundle that doesn't embed all the linked resources, Krita will use the latest version of the dependent resource. Creation of such bundles is not recommended.

As far as I can tell, satisfying these requirements is possible in the current design of the resource system, though it would be much more difficult and error-prone than if we just linked to the resource-version-id all the time. We will have to manually maintain consistency of the database and (most probably) include all the dependency information into it. But, as I said before, I don't mind any implementation, given the requirements above are satisfied.

The most important thing for me is that these three behaviors would be considered as a release-blocker-level bug:

  1. The bundle is imported and the brushes inside it look different to how they looked at the creator's PC.
  2. Importing a bundle changed how the brushes in the resources folder look.
  3. While overriding a resource Krita silently changed the look/behavior of the dependent resources.

And I repeat again, I'll agree to any design that fits these requirements.

Hi, I had difficulties to follows the full thread and I tried my best. At one point, I lost it (probably English + technical terms).
So, I prepared a tiny scenario with four panels (A, B, C and D) to storytell a situation, then 4 questions. Please enlarge the image attached under to read it.
I hope it will help to check if the current design + requirements can solve these usecases.
My apologies if I bring here too basic scenarios or problems considered out of the scope (in this case, just tell me and I'll fly away 😉).
Thank you for the hard work.

Hi, @Deevad!

Thank you for nice usecases! I will try to model how the system will behave with all three implementation options we have atm:

Linking by resource-version-id

All resources are linked to specific versions of the resources. So during the update, the look of the dependent resources will not change.

[1] After update "star necklace V1" preset will look in exactly the same way as before the update. To update the preset Carrot should enter the preset editor, select the new "Star v2" brush tip and press "Overwrite" button. That will effectively create a new version of the preset called "star necklace V2" .

[2] To revert the change Carrot should just revert preset version from "star necklace V2" to "star necklace V1"

[3] After update Yuzu's preset "star stroke V1" is unchanged. Yuzu just continues to use it as before.

[4] Yuzu can use official "many star v2" alongside with "star stroke V1" without doing any additional steps.

Dynamically link by resource-id

The problem is that for some reason Halla doesn't like "linking by resource-version-id" approach and forces the following design:

Resources are linked to "the latest version of the resource". That is, during update all the resources linking to it will automatically use the updated version.

[1] After update Carrot will see that "star necklace V1" automatically uses "Star v2".
[2] To revert "star necklace V1" to use "Star v1", Carrot will have to revert the whole "Star v2" brush tip to the older version. That will also revert "Many stars v2" preset to use older brush tip. If "Many Stars v2" had any other changes than the change in the brush tip, then the preset will effectively become "broken" (it will be neither "Many Stars v1" nor "Many Stars v2").
[3] Impossible. Yuzu cannot reject update. Their "star stroke v1" preset will use "Star v2" brush
[4] Impossible. Yuzu will have to somehow get the older version of the brush tip, save it under a different name and manually modify the "star stroke v1" to use that differently named resource.

Compromise approach

After more than a month of discussion, I proposed a compromise approach, which tries to resolve some problems of the previous design. Even though I'm not happy with that, I consider that as an "acceptable" solution.

This approach is basically "dynamically linking by resource-id", but with some restrictions on the bundles that I listed in this post. Your usecases will look like that if we accept that:

[1] After update Carrot will see that "star necklace V1" automatically uses "Star v2".
[2] To refuse the update, Carrot will have to export "star necklace V1" into the bundle before the update. Later, when the bundle will be imported after the update, brush tip "Star v1" will appear in the list of the resources and will duplicate an older version of "Star v2". It might also happen that Carrot will end-up with two copies of "star necklace V1" resource, one linking to the new brush tip, the other to the old one. It depends on the particular implementation.
[3] Yuzu will have to export the preset into a bundle before the update and import it back later, like in the point above.
[4] After importing the pre-saved bundle, the list of brushes will show both tips (but with the same name and filename). These brush tips will have separate history. Yuzu can use both brushes (if they mange to distinguish them :) )

Thank you very much @dkazakov for the detailed post. 👍

Among these models, "Linking by resource-version-id" (the first one in you list) would be the one I prefer (by far) since it succeeds already scenario [2],[3] and [4].

One major weakness, tho, in scenario [1]: the update of the brush tip from Star(v1) to Star(v2) means Carrot would need to follow Krita blog to be notified the Star brush tip changed, and then take action in brush editor to DIY his own update... It sounds too difficult and I predict many users will never benefit new update of resources in bundles with a system like that.

I would prefer −if possible− Carrot to be prompted after the update with two dialogs (one for brush preset "star necklace" and another for brush preset "Many stars") with buttons to auto-update the presets or skip the update. (and a sentence to friendly say it can be reverted later in brush editor. So, it will be easier for Carrot to know how to revert later if he accept and immediately dislikes the new Star.)

Hi, @Deevad!

One major weakness, tho, in scenario [1]: the update of the brush tip from Star(v1) to Star(v2) means Carrot would need to follow Krita blog to be notified the Star brush tip changed,

Well, at least original "Many Stars" preset will automatically updated to use the new version of Star(v2), because Carrot never changed "Many Stars" resource. But his custom resource will link to the old one yes.

I would prefer −if possible− Carrot to be prompted after the update with two dialogs (one for brush preset "star necklace"

That is possible to do.

and another for brush preset "Many stars"

I'm not really sure it is needed unless Carrot created a custom version of this preset. Are you sure such notifications are needed for resources that didn't have custom versions created by the user?

Are you sure such notifications are needed for resources that didn't have custom versions created by the user?

Yes. to protect users who really likes their preset "as it is" for their workflow, even if they never made any custom version of it and use the vanilla preset unchanged.

Please WAIT a bit until I've read everything that has been posted before coming to conclusions...

Because from skimming, all y'all are doing is going into specifics withough coming up with anything general.

And BESIDES I am the one working on this, NOT Dmitry. He can have ideas and suggestions, but I will implement what I think best.

@rempt Sorry about that. It was only scenarios and feedback about my preferences. Not a will to put an order, command or to impose a vision about a design. Again, sorry if I wrote this way, I had no idea about it. I'll move away from this topic to not add more noise, but I'll be always around if you need any feedaback, test, scenario or usecase.

The reason this discussion has dragged on so long is basically that I am not trusted to design and write code once Dmitry has touched any part of that. And, yes, this makes me very unhappy and very tired. I am not intererested in a compromise: I want to have the trust to be allowed to work on the application I am the maintainer of.

So, let's get back to basics, use generic terms and hope that this time I'm getting through.

Let's NOT talk about bundles as if they were something special.

Definitions

  • Storages

Storages are things where resources can be stored. Storages can be moved from computer to computer. Examples are bundles, folders and .kra files. (Yes, we always intended to make the folder movable, and it is, in fact, possible to have different resource folders on the same system, although Krita can use only one of them at a time.)

A storage is defined by its location: for a bundle, that's the bundle's filename. MyBundlev1.bundle is a different storage than MyBundlev2.bundle.

  • Resources

Resources are things that can be stored in a storage. Resources have a type. Within a given storage, within a given type, there cannot be two resources with the same filename.

If there is a resouce with the same filename and the same type in two different storages, it is not the same resource.

Only if the file contents are also identical, one of those resources is hidden for the user. The resource still exists in the database and can be accessed.

  • Dependent Resources

Those are resources that another resource needs to function. Dependent resources are either inside the storage of the owner resource, or not.

  • Resource Versions

If a resource is modified and saved, a new version of that resource is created. It is still the same resource, so everything that uses that resource will use the new version. The only reason we added resource versioning is so the user can go back to older versions of the resources they have modified.

Linking Resources

As said, a dependent resource can be either in the same storage as the owner, or not:

  1. Dependent resource and owner resource are in the same storage

This can be because:

  • The user creates a bundle: all resources save their dependent resources to the bundle. This is specific for a bundle, and doesn't (yet) work for the folder storage, or more importantly, .kra files.
  • The user saves the resource to the storage that also contains the dependent resource.
  1. Dependent resource and owner resource are not in the same storage
  • the user created a resource that uses another resource but saves the resource to a different storage
  • the user modified a resource to use a resource from another storage
  • the storage was delivered incompletely

Modifying Resources

Modifying a resource creates a new version. Everything that uses that resource, should use the new version.

Replacing Storages

If a storage is replaced by a storage with the same filename, the contents are synchronized.

We had this table in our original design document:

User made no changeUser changedUser Removed
Krita made no changeNo change
Krita changedCopy to userKeep user, no historyKeep remembered
Krita remove simpleRemoveKeep
Krita force removeRemove even when used a lotKeep
Krita addAdd resource

This talks about updating Krita with a new set of resources, but that also specifies how to handle replacing storages with a new version of that storage. So, we did specify that if a storage is replaced, the user's versions would be kept and used.

This should have the following effect:

  • Resources with the same md5sum, type and filename are not changed in the database. If there are new versions of that resource created by the user, those new versions are still used.
  • Resources that do not occur in the storage but do occur in the database and have user versions are kept.
  • Resources that do not occur in the storage but do occur in the database and do not have user version are removed from the database.
  • Resources with a new md5sum are added to the database.

This means that a certain storage now can have two different resources that started out as the same one: the user's version and the storage's version. Because after this, they are no longer the same thing.

This means that updating a bundle doesn't somehow save the old version of a resource as a prior version for the new resource. We're not keeping different versions of resources that came with a storage as if they were user-modified versions.

For resources in another storage that use a resource in the replaced storage that has been deleted, we will have to have a fallback by filename. And yes, that means that the user's local resources will use the new resource.

Answering Deevad's Questions

  1. By updating to the new version of the bundle
  2. By downgrading to the old version of the bundle, or copying the old version of the brush tip to the folder storage and using that in their presets
  3. By copying the brush tip into the folder storage, and using that tip
  4. Same thing

Dmitry's "Release Blockers"

Note: "importing" is probably used by Dmitry in two ways, adding a new bundle and overwriting an existing bundle of exactly the same name.

1a. If the bundle is complete and new, yes, that works.

1b. If the bundle is complete and replaces an existing bundle from which the user has modified a resource, but the resource is the same in the new bundle as in the old bundle, the modified version of the resource should be used, because it's the same resource, and the user has modified it. (NOTE: this is pretty hypothetical and currently only possible for gradients and presets. In most cases, users will save modified resources under a new name.) This is exactly what we specified in https://phabricator.kde.org/T379, the user's versions of resources are used.

  1. It depends. Adding a storage shouldn't do a thing. Replacing a storage means that the new version of the resource should be used, or that the resource
  1. that sentence makes no sense?

My Conclusion

I'm implementing this as designed in https://phabricator.kde.org/T379. We did not design this system to create versions of resources in storages that are replaced by another version of that storage. That whole idea is out of scope.

The reason this discussion has dragged on so long is basically that I am not trusted to design and write code once Dmitry has touched any part of that

That is not a question of trust. There is a user workflow that either works or is broken. I don't question your position of the maintainer, you are doing a good job in general. I question one specific design decision of yours, which is wrong. And two separate painters have already confirmed that your decision is not what they expect.

I'm implementing this as designed in https://phabricator.kde.org/T379

I told you about these design problems back in 2015 during the discussion in Deventer. You just forced me to keep silence and ignored any issues I was talking about. I though you will handle these issues at least somehow. Five years passed, I waited till you come up with something. Now I "touched any part of" the code, and saw that you did nothing to handle them! Just nothing! The current system will happily destroy user presets in numerous cases (which we listed multiple times already).

And instead of leading an objective discussion and listening to what four different people are telling you, you just are just doing personal attacks on me. That is not good.

dkazakov added a comment.EditedMay 24 2021, 2:35 PM

Now on the technical topic:

Answering Deevad's Questions

That is exactly what I described in the "Dynamically link by resource-id" section. The user basically has to recreate all his/her presets from scratch because they became broken by the update. That is not what either @Deevad or @kamathraghavendra expect from the resource system.

Dmitry's "Release Blockers"

1a. If the bundle is complete and new, yes, that works.

Right now it is broken. Here is an illustration. It breaks the second requirement.

1b. If the bundle is complete and replaces an existing bundle from which the user has modified a resource

I don't talk about replacing bundles. I talk about separate bundles.

  1. It depends. Adding a storage shouldn't do a thing. Replacing a storage means that the new version of the resource should be used, or that the resource

Okay, it is good that we agree at least in something. I don't talk about "replacing". I talk only about adding a new bundle with a overridden resource from the shared source. Now it is only left to make the system actually follows this rule :)

  1. that sentence makes no sense?

While overriding a resource Krita silently changed the look/behavior of the dependent resources.

Here is an illustration:

Yuzu overrides Star v1 resource in the resource folder. Krita must ask him/her what to do with dependent resources. Yuzu chooses to override Many Stars and not to override USSRNecklace. Krita creates a new version of Many Stars resource and removes disability flag from the "Star v1" resource inside the bundle.

What I wanted to tell with this sentence is that no (dependent) resource must be changed without explicit user's consent.

rempt closed this task as Resolved.May 31 2021, 10:05 AM

Okay, we're clearly not going to come to an agreement here based on the premises of this discussion. I don't want an already over-complicated system to get even more complicated in sight of the finish line, and Dmitry isn't going to allow me to implement the original design.

However, a discussion I had with Tiar last week gave me an idea on how to break the deadlock.

In the end this discussion is pretty much focussed on dependent resources for paintop presets. We originally implemented this system for two reasons:

  1. If a dependent resource would change, presets would automatically use the new version
  2. to save memory when loading presets on startup.
  1. is obviously no longer a requirement since this whole argument is about always using the version of a resource the preset was created with.
  2. is no longer relevant since we don't load all presets on startup anymore.

So, let's just get rid of the whole dependent resource fetching system for presets and embed patterns, brush tips and gradients directly in the .kpp file and load them from there. This will significanly simplify our code and make brush presets much more robust.

This will also make it easier to fix the brush preset editor to not use the brush tip, gradient or pattern that gets accidentally selected in the option page.

Hi, @rempt!

Thanks a lot for switching into constructive discussion mode! In general I agree that this proposal will solve the issue. Though I have two questions and one problem that I would like to discuss:

Questions:

  1. Do I understand it right that now we should make sure that all our resources embed their dependencies inside? Including filters and generators?
  2. Do you have an idea about a timeline for that? What part of this work should be done before the Krita 5 release and what part can be postponed. This proposal involves a file-format change, so it would be good to think about backward compatibility beforehand.

Problem:

> brush tips and gradients directly in the .kpp file and load them from there

The main reason why we needed to link to the resource in the Krita resource folder was not only to save memory, but to be able to show this resource in the resource selector. How do you propose to solve this issue? I see two approaches:

  1. Always show the currently embedded resource separately in the resource chooser. It may lead to a small resource duplication in the GUI selector. That is not a critical issue, but still...
  2. Still try to find correspondence of the embedded resource with the one in the database using md5 (we will have to save this "original md5" separately into the container-resource then). We use some a variant of this behavior in KisEmbeddedPatternManager, but it will have to be modified a bit (remove fetching by name and filename).

What do you think is the best/simple solution for that?

  1. Do I understand it right that now we should make sure that all our resources embed their dependencies inside? Including filters and generators?

For these, I want to give the user the option to include the resource in the document storage when creating the layer or mask, because of the usecase where, say, a gradient is used on a number of pages and the user wants to change the gradient for all the pages..

  1. Do you have an idea about a timeline for that? What part of this work should be done before the Krita 5 release and what part can be postponed. This proposal involves a file-format change, so it would be good to think about backward compatibility beforehand.

I want to get started now, but I need to figure out backwards compatibility. Either keep the current code that works as in Krita 4 for a while, or add a bundle converter option.

The main reason why we needed to link to the resource in the Krita resource folder was not only to save memory, but to be able to show this resource in the resource selector. How do you propose to solve this issue? I see two approaches:

  1. Always show the currently embedded resource separately in the resource chooser. It may lead to a small resource duplication in the GUI selector. That is not a critical issue, but still...

This is what I want, because it will solve the bug where the if the brush tip is not available anymore it will be replaced by one in the resource selector. So I want to make changing the embedded resource an explicit action.

  1. Still try to find correspondence of the embedded resource with the one in the database using md5 (we will have to save this "original md5" separately into the container-resource then). We use some a variant of this behavior in KisEmbeddedPatternManager, but it will have to be modified a bit (remove fetching by name and filename).

So, no, I don't want to do this.

a gradient is used on a number of pages and the user wants to change the gradient for all the pages..

It sounds like that "shared resources" feature in ToonBoom. We would still need some framework for loading dependent resources then... Anyway, if the user needs to mark these resources as "shared" explicitly, I'm perfectly fine with that.

Btw, it will also solve a many-years-long feature request to make a per-document texture for brushes.

So, no, I don't want to do this.

Well, I would really like to have that (at least combined with the first option). It might make GUI a bit simpler in some cases (unless you want to have a very special GUI for the "currently embedded preset"). But I won't insist on that.