paintops

[Schleimer, Ben]

Hi Boud,
  There is a problem with using just KisPaintInformation to transfer operation properties from the
tool to the paintop. In krita 1.6, KisPainter has methods set/duplicateXXX and set/pressure hacked
in because KisPaintInformation doesn't carry the required info. I suggest making
KisPaintInformation more dynamic: 

1) instead of hardcoded information, make it a QString->QVariant hash (the paint "properties") so
instead of KisPaintInformation::pressure, it would be KisPaintInformation["pressure"].
2) the tool populates the properties with as many properties as it has ("tilt", "pressure",
"duplicateOffset", "duplicateHealing", etc.
3) The KisPaintInformation is passed to the KisPainter->KisPaintOp for each operation. The PaintOp
checks for each property it is interested in and if it's not in the hash, it uses a default value:

void DuplicatePaintOp::paintAt(const QPoint& pt, const KisPaintInformation & info)
{
  double pressure = 1;
  if(info.has("pressure")) { pressure = info["pressure"]; }
  double tilt = 0;
  if(info.has("tilt")) { tilt = info["tilt"]; }
....
}

Pros:
It'll be cleaner to use then the current hardcoded setup.

Cons: 
Efficiency - if populating and reading the hash with tilt/pressure values each time is too slow,
then we could use a int->QVariant or int->int lookup. Additionally, we could have a enum of common
properties that the tools and paintops can share. Also, most paint properties only have to be
written and read from the hash only the first time the tool is used.

What do you think?
Ben

PS. an alternative is to for the tool send an inherited class of KisPaintInfo and have the paintOp
use dynamic_cast<> to try to get the correct type.

PSPS. Another alternative is for the tool to populate the properties of the paintOp directly
instead of going through KisPainter. This means the tool needs to do dynamic_cast on the paintOp.

--- Boudewijn Rempt <boud at valdyas.org> wrote:

> Hi,
> 
> Thanks to Hulmanen's patience we've uncovered a design flaw in our paintops.
> When I designed paintops way back in the old days, I didn't realize that when 
> you have pressure settings the spacing of the brush footprints would change. 
> Or rather, when you have pressure but don't change the size of the brush 
> footprint, the spacing should not change. This is a problem, because the 
> spacing of the brush footprints is not under control of KisPaintop but of 
> KisPainter::paintLine.
> 
> For 2.0, we have to do better. There are other types of paintops (bristle 
> simulating brushes spring to mind) where the paintop needs to have control 
> over the line that needs to be painted between two mouse/tablet events (note 
> that mouse/tablet events never occur at the right spacing: we always have to 
> either in-between, hence paintLine, or skip a few, if the brush is too 
> large).
> 
> paintLine is the only (?) method in KisPainter that is called when doing 
> freehand work that involves tablet settings. 
> 
> The rect, star etc. tools don't do pressure, I believe. So the 
> KisPainter::paintLine in-betweening code is fine for those.
> 
> I've got the following design proposal, but I don't claim it's the best:
> 
> add a KisPaintop::paintLine(QPointF pos1, QpointF pos2, const 
> KisPaintInformation& info)
> 
> This method _can_ call KisPainter::paintLine, which calls the paintops's 
> paintAt method repeatedly. Or it can do something new and wondrous to 
> in-between.
> 
> The freehand tools then call KisPaintop::paintLine instead.
> 
> Any better ideas?
> 
> -- 
> Boudewijn Rempt 
> http://www.valdyas.org/fading/index.cgi
> > _______________________________________________
> kimageshop mailing list
> kimageshop at kde.org
> https://mail.kde.org/mailman/listinfo/kimageshop
>