Looks like a release schedule to me

[Casper Boemann]

Wice words all the way through.

I have taken some time and thought this through (I hope):

1) for layers it would be ok to do like your original patch

2) for paintdevices that use the offset for other things (like hotspots in 
brushes) it would probably not be a good idea.

3) I don't think anyone is using paintdevices like in 2)

4) a paintdevice with offset 0,0 behaves like now

So what it all boils down to is that the original patch would be ok. Remember 
that I never said I had any concrete arguments against it - just a bad 
feeling.

From your new idea I like the thing about virtual creators of iterators and 
overload from layer. That was the point that helped me come to my conclusion. 
eventhough as I explain next I don't think it is needed.

I think I would like if we moved offset to layers at the same time we apply 
your patch. We don't have to do it virtualy however. A faster way would be to 
just move the setOffset functions to layer, but keep the actual variables in 
paintdevice (as protected). That way only layers can be moved.

In the new idea (as you said yourself) storing and updating variables in 
iterators would be a hazzle, and I believe: not worth it.

So ok from me to apply the original patch (havn't looked at the actually code 
though). I still have this (unsupported) nagging feeling however, but unless 
you are yourself still in doubt, I think it would be ok.

Again, thanks for the many wice thoughts. They really helped me think this 
through.

On Sunday 13 March 2005 22:16, Bart Coppens wrote:
> On Sunday 06 March 2005 21:12, Casper Boemann wrote:
> > I don't think it is the right place and way to fix this. At least
> > extend() needs to be fixed in the same way then (I havn't looked at the
> > code if you have already done that)
>
> Well, I fixed the paint device's extent() in that patch (the wrong one,
> though, but that's not that important as it was a quick try)
>
> > But more fundamentally it hides the layer offsets in a way that may be
> > confusing and problematic in the future. It does thing behind our backs
> > and that worries me.
> > Remember that iterators must also work on paintdevices. You patch does
> > work on paint devices, but I would like the (layer)offset to be moved to
> > the layer class, and then your fix would be a problem.
>
> Then what would you think of something like the following. (This is just an
> idea, I haven't checked if it's actually possible or how much work it would
> take.) Instead of letting the iterators store a pointer to a paint device,
> and get the offsets from there, let them store the offsets themselves.
>
> Then you could let the iterators use the offsets of the layer by default,
> but still have the ability to return to the original coordinates by use of
> a setOffset(int,int) method of the iterator (this might be a problem,
> though, as the iterators would have to switch to the right tile and so
> after changing the offset).
>
> If the offset functionality would then be moved to the layers, the
> iterators of the regular paintdevice might set the offsets to 0, but the
> (virtual) createiterator methods of the layer could make them with correct
> offsets. The same goes for the extent function.
>
> This way, it's hidden for regular use, but not out of reach if it's really
> needed.
>
> > The fact that we must also change extent to make your fix work, suggest
> > to me that it is too complicated. (eventhough the idea and implementation
> > is simple)
>
> That's true, I must admit :-(, and the idea above is even more complicated
> codewise.
>
> > So my suggestion would be to let the layerclass have coordinate
> > transforming functions, and to transforn the coordinates on an as-needed
> > basis inside tools, plugins etc.
>
> Now I seem to remember a quote about APIs, that goes something like 'make
> the common task easy to do, instead of making the uncommon easy and the
> common difficult' (or something like that). It seems to me, that a plugin
> almost always needs to take the offsets into account, whereas some code
> that doesn't would probably be quite rare.
> I think it would be more productive to let the more common way of using the
> Krita layers be more compact than the rare way: it's not only shorter on
> typing, but also safer. A (beginning) programmer writing Krita code would
> more easily forget adding those transforms if they were required for
> everything, whereas a coder making something that explicitly would need
> them, has most probably not forgotten to think of this issue.
>
> Bart Coppens
> _______________________________________________
> kimageshop mailing list
> kimageshop at kde.org
> https://mail.kde.org/mailman/listinfo/kimageshop

-- 
best regards / venlig hilsen
Casper Boemann