Renaming KisPainter::bitBltFixedSelection to bitBltWithFixedMask

LukasT.dev@gmail.com lukast.dev at gmail.com
Thu Sep 9 13:56:22 CEST 2010


On Thursday 09 September 2010 10:10:13 JL VT wrote:

> The former bitBltFixedSelection used to override the user selection
> (d->selection) with a fixed selection provided as a parameter. But
> nobody used it intending to override the user's selection with that
> new KisFixedPaintDevice selection, instead it was used as a mask to
> give a brush its characteristic shape (that was the case with the
> SmudgeOp and DuplicateOp, and later my own HatchingOp).

I need to use something similar in deform to be able to fix bug
https://bugs.kde.org/show_bug.cgi?id=217124

but I think I need something else, not merging but intersection of the 
selecitons. There is method:

void KisPainter::bltFixed(qint32 dx, qint32 dy,
                          const KisFixedPaintDeviceSP srcDev,
                          const KisFixedPaintDeviceSP selection,
                          qint32 sx, qint32 sy,
                          qint32 sw, qint32 sh)

Which has the same problem with selections.
It overrides selection specified by user. I'm working on it.
I think that maybe the Composite operation might be parameter of that method 
to define strategy to merge / intersect selections.

> There is a function in the test kis_painter_test.cpp,
> KisPainterTest::testSelectionBitBltFixedSelection(), that tests
> whether an arbitrarily bounded source QRect turns into the expected
> destination QRect when blitting. However this test makes no sense
> anymore, since when using the fixed selection as a mask for a brush
> dab, the most sensible way to use it is to have a mask of the same
> size than the QRect to be extracted from the dab, anything else leads
> to either unpredictable or unclear behavior, and it goes against code
> readability. So I gladly followed Dmitry's suggestion of putting an
> ASSERT to check whether someone tries to use bitBltFixedSelection in
> this unwieldy way (a source QRect of different size than the mask).
> This very ASSERT immediately broke the now clearly obsolete unit test.

There is one problem. The user specify sx, sy and current status of the code 
expect that sx and sy are 0,0. I'm not sure if we can leave it this way.
Either fix that for behaviour where sx is in 0 <= sx, sy <= sw,sh or do 
something like don't allow to specify them?

 
> Therefore, the changes I propose are:
> 1.- rename bitBltFixedSelection to bitBltWithFixedMask to reflect what
> the function does now (which was in fact what people were using it
> for).

Mask? bitBltWithFixedSelection? 

> 2.- eliminate the obsolete unit test (
> KisPainterTest::testSelectionBitBltFixedSelection() ).

I would propose you to write some simple test that test the behaviour so that 
we can catch regressions.



More information about the kimageshop mailing list