[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-kimageshop
Subject:    Re: Painting on selections and selection masks
From:       Boudewijn Rempt <boud () valdyas ! org>
Date:       2011-10-29 8:03:38
Message-ID: 201110291003.38329.boud () valdyas ! org
[Download RAW message or body]

On Friday 28 October 2011 Oct, Dmitry Kazakov wrote:
> > > > colorspaces can convert the pixels from another colorspace to their own
> > > > before doing composition, but for painting on masks that's not
> > currently
> > > > useful since we generate dabs in the target colorspace anyway.
> > > > 
> > > 
> > > Well, the colorspace of a dab is really not a problem, i guess. A node
> > can
> > > have both paintDevice() and dabColorSpace() methods. And tools can use
> > the
> > > latter one to create a dab of a proper colorspace.
> > 
> > I'm not convinced that we're just having problems with dabs -- I think it's
> > a more general problem, that shows up everywhere in krita where we
> > manipulate paint devices.
> > 
> 
> Could you make a couple of examples? I don't get it.

The hairy brush doesn't create dabs, the fill tool doesn't create dabs, the gradient \
tool doesn't create dabs -- and they all need an alpha channel to work correctly.

> > And there's no need to actually solve it at the node level, that's too high
> > up.
> 
> 
> I don't think this is too high up. A node reports which colorspace you
> should paint *on it* with. Probably, even usual colorSpace() method is
> capable of doing this, but I'm not sure it'll work. It needs thorough
> investigations.

We don't paint on nodes, we paint on paint devices. Adding api for painting to \
KisNode is certainly wrong.

...

> As I said the api of the KisPaintDevice shouldn't (and mustn't) be changed.
> 

Sorry, then your solution will not work. KisNode is the wrong place, since we do not \
paint on nodes.

> > > I'm sure we shouldn't hurry with such decisions. Especially, after we
> > have
> > > released several betas.
> > 
> > No -- but not being able to properly paint on selections and masks is a
> > critical bug, which is why I have tried to find a minimally invasive way to
> > make it possible, mostly contained in KisSelection. Unfortunately it means
> > we triple memory usage for selections, so we have to think whether that's
> > worth it.
> > 
> 
> Well, I don't think that KisSelection is a proper place for doing this.

Why not? That is the interface between things to paint on (the mask's paint device) \
and the selection itself (the selected bytes). It's the only point where the two \
assumptions Krita is built on come together: painting needs an alpha channel, masking \
needs a single byte per pixel.

> As I
> can see in your patches, you copy bytes from one colorspace to the other
> inside KisSelection.

Yes -- just like it would happen anyway if you have a vector selection. That also \
copies bytes into the projection.

> So you do the conversion actually. Isn't it exactly
> what pigment is supposed to do?

I did not use pigment's color conversion code here because it would be faster to just \
copy the bytes, but it is possible. The conversion would then happen when bitBlting \
the KisPixelSelection onto the KisSelectionProjection. No real difference, in my \
opinion.

> And I don't even speak about the ability to
> use 8/16 bit selection masks in the future.

Well, that's a lot easier to do if everything is contained inside KisSelection. \
Though I don't think we'll ever do 16 bit selections, no user has ever asked for it.

> 
> And triple memory usage is hardly acceptable as well.

That is the only big problem I see. But given that this is a critical bug for the 2.4 \
release, I think it could be acceptable. There is no other way to fix the problem.

-- 
Boudewijn Rempt
http://www.valdyas.org, http://www.krita.org, http://www.boudewijnrempt.nl
_______________________________________________
kimageshop mailing list
kimageshop@kde.org
https://mail.kde.org/mailman/listinfo/kimageshop


[prev in list] [next in list] [prev in thread] [next in thread]