From koffice-devel  Wed Jul 29 20:46:37 2009
From: "Dmitry Kazakov" <dimula73 () gmail ! com>
Date: Wed, 29 Jul 2009 20:46:37 +0000
To: koffice-devel
Subject: Re: Review Request: Refactoring for KisPrescaledProjection
Message-Id: <20090729204637.3838.47044 () localhost>
X-MARC-Message: https://marc.info/?l=koffice-devel&m=124890041113504



> On 2009-07-29 20:16:28, Casper Boemann wrote:
> > I don't enough energy to look at your code really thoroughly. I'm sorry, but I don't think i can approve, because the special logic you have removed ensured that the subpixel positioning of the scaling coded worked.
> > 
> > Unless you can convince me that you have completely understood that and have somehow worked around that problem I don't think this should go in.
> > 
> > but again I've only had time to look at it loosely, so please convince me otherwise
> 
> Dmitry Kazakov wrote:
>     Subpixel positioning. Do you mean the one was used after Blitz::smoothScale(..)? This positioning is done in a special function - drawWithRoundingCorrection(...). I'm gonna use this function with the pyramid, so nothing is changed here, just made in a separate function.
>     
>     Alignment to KisImage's pixels hasn't changed much either, it's done through QRect(imageRectFromViewPortPixels()) --> viewRectFromImagePixels() chain. The only change here (not sure this is a change, it was so before, i guess) - result viewRect can be slightly bigger than the canvas due to alignment. But QPainter works it over very well. More than that, such behavior was present before this patch and it worked quite well.
>     
>     The thing that was really removed is "useNearestNeighbour" option. I geuss, it's quite deprecated, but i'm gonna implement it somehow later.
> 
> Casper Boemann wrote:
>     Hmm Ok as far as I'm concerned you have proved to be on top of it. I still havn't read you commit exactly, so I can't really say go, but I have no objections anymore

2all:

Maybe someone knows some special testcases of subpixel positioning?

I have tested it much: with different documentOffset's and large scales. Painting works.


- Dmitry


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/1156/#review1844
-----------------------------------------------------------


On 2009-07-29 11:44:40, Dmitry Kazakov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/1156/
> -----------------------------------------------------------
> 
> (Updated 2009-07-29 11:44:40)
> 
> 
> Review request for KOffice.
> 
> 
> Summary
> -------
> 
> Makes prescaled projection code simplier and clearer. It also removes weird [1], simply unused [2] and previously unused code with bugs [3].
> 
> Most significant changes:
> 1) imageRectFromViewPortPixels and viewRectFromImagePixels now just convert rects' coords, without respect to canvas size (they still do respect image size). 
> 2) They both made _private_ so this change won't brake anything outside
> 3) Broke the link from KisCanvas2 to KisPrescaledProjection::viewRectFromImagePixels. It was quite bad idea to use internal conversion function from outside. Reasons:
>     a) they are internal
>     b) vRect can be changed (expanded) by prescaled projection during update (mipmap case)
>     c) viewRectFromImagePixels was called twice: from KisCanvas2 first and then from KisPrescaledProjection::updateCanvasProjection again
> 4) now it's easier to maintain
> 
> 
> [1] kis_pescaled_projection.cc:412-431
> [2] kis_pescaled_projection.cc:396-403
> [3] kis_pescaled_projection.cc:532
> 
> 
> Diffs
> -----
> 
>   trunk/koffice/krita/ui/CMakeLists.txt 1000698 
>   trunk/koffice/krita/ui/canvas/kis_canvas2.cpp 1000698 
>   trunk/koffice/krita/ui/canvas/kis_prescaled_projection.h 1000698 
>   trunk/koffice/krita/ui/canvas/kis_prescaled_projection.cpp 1000698 
>   trunk/koffice/krita/ui/tests/kis_prescaled_projection_test.cpp 1000698 
> 
> Diff: http://reviewboard.kde.org/r/1156/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Dmitry
> 
>

_______________________________________________
koffice-devel mailing list
koffice-devel@kde.org
https://mail.kde.org/mailman/listinfo/koffice-devel