Fix rounding in wrap around mode for KisPainter
ClosedPublic

Authored by alvinhochun on Nov 19 2017, 4:41 PM.

Details

Summary

Here I tried to correct some of the rounding behaviour of KisPainter which would produce erroneous results for negative coordinates, most of which are truncating floating points instead of flooring them. I also removed some redundant conversions and rounding. Some of the code already does nearest rounding so I did not touch them.

There shouldn't be any regressions regarding positive coordinates, but I can't tell if this fixes all the negative coordinates rounding bugs of KisPainter. All I can tell is that this fixes the pixel brush offset (https://bugs.kde.org/show_bug.cgi?id=367943).

Test Plan

Pixel brush with sharpness (pixel1) doesn't work on master, so it has to be tested with krita/3.3. P137 is a version of this patch which can be cleanly applied on krita/3.3.

1commit b4d0e46ea81503b3bf5db43383952ffeb491fc42
2Author: Alvin Wong <alvinhochun@gmail.com>
3Date: Mon Nov 20 00:07:55 2017 +0800
4
5 change rounding method
6
7diff --git a/libs/image/kis_painter.cc b/libs/image/kis_painter.cc
8index 1e7a5be599..eac1b06d2c 100644
9--- a/libs/image/kis_painter.cc
10+++ b/libs/image/kis_painter.cc
11@@ -73,10 +73,6 @@
12 // Maximum distance from a Bezier control point to the line through the start
13 // and end points for the curve to be considered flat.
14 #define BEZIER_FLATNESS_THRESHOLD 0.5
15-#define trunc(x) ((int)(x))
16-#ifndef Q_OS_WIN
17-
18-#endif
19
20 struct Q_DECL_HIDDEN KisPainter::Private {
21 Private(KisPainter *_q) : q(_q) {}
22@@ -1547,10 +1543,10 @@ inline void KisPainter::compositeOnePixel(quint8 *dst, const KoColor &color)
23
24 /**/
25 void KisPainter::drawLine(const QPointF& start, const QPointF& end, qreal width, bool antialias){
26- int x1 = start.x();
27- int y1 = start.y();
28- int x2 = end.x();
29- int y2 = end.y();
30+ int x1 = qFloor(start.x());
31+ int y1 = qFloor(start.y());
32+ int x2 = qFloor(end.x());
33+ int y2 = qFloor(end.y());
34
35 if ((x2 == x1 ) && (y2 == y1)) return;
36
37@@ -1636,11 +1632,11 @@ void KisPainter::drawLine(const QPointF & start, const QPointF & end)
38
39 void KisPainter::drawDDALine(const QPointF & start, const QPointF & end)
40 {
41- int x = int(start.x());
42- int y = int(start.y());
43+ int x = qFloor(start.x());
44+ int y = qFloor(start.y());
45
46- int x2 = int(end.x());
47- int y2 = int(end.y());
48+ int x2 = qFloor(end.x());
49+ int y2 = qFloor(end.y());
50
51 // Width and height of the line
52 int xd = x2 - x;
53@@ -1702,19 +1698,19 @@ void KisPainter::drawDDALine(const QPointF & start, const QPointF & end)
54
55 void KisPainter::drawWobblyLine(const QPointF & start, const QPointF & end)
56 {
57- KisRandomAccessorSP accessor = d->device->createRandomAccessorNG(start.x(), start.y());
58+ KoColor mycolor(d->paintColor);
59+
60+ int x1 = qFloor(start.x());
61+ int y1 = qFloor(start.y());
62+ int x2 = qFloor(end.x());
63+ int y2 = qFloor(end.y());
64+
65+ KisRandomAccessorSP accessor = d->device->createRandomAccessorNG(x1, y1);
66 KisRandomConstAccessorSP selectionAccessor;
67 if (d->selection) {
68- selectionAccessor = d->selection->projection()->createRandomConstAccessorNG(start.x(), start.y());
69+ selectionAccessor = d->selection->projection()->createRandomConstAccessorNG(x1, y1);
70 }
71
72- KoColor mycolor(d->paintColor);
73-
74- int x1 = start.x();
75- int y1 = start.y();
76- int x2 = end.x();
77- int y2 = end.y();
78-
79 // Width and height of the line
80 int xd = (x2 - x1);
81 int yd = (y2 - y1);
82@@ -1735,8 +1731,8 @@ void KisPainter::drawWobblyLine(const QPointF & start, const QPointF & end)
83 y = y + inc;
84 x = qRound(fx);
85
86- float br1 = int(fx + 1) - fx;
87- float br2 = fx - (int)fx;
88+ float br1 = qFloor(fx + 1) - fx;
89+ float br2 = fx - qFloor(fx);
90
91 accessor->moveTo(x, y);
92 if (selectionAccessor) selectionAccessor->moveTo(x, y);
93@@ -1762,8 +1758,8 @@ void KisPainter::drawWobblyLine(const QPointF & start, const QPointF & end)
94 x = x + inc;
95 y = qRound(fy);
96
97- float br1 = int(fy + 1) - fy;
98- float br2 = fy - (int)fy;
99+ float br1 = qFloor(fy + 1) - fy;
100+ float br2 = fy - qFloor(fy);
101
102 accessor->moveTo(x, y);
103 if (selectionAccessor) selectionAccessor->moveTo(x, y);
104@@ -1787,19 +1783,18 @@ void KisPainter::drawWobblyLine(const QPointF & start, const QPointF & end)
105
106 void KisPainter::drawWuLine(const QPointF & start, const QPointF & end)
107 {
108- KisRandomAccessorSP accessor = d->device->createRandomAccessorNG(start.x(), start.y());
109- KisRandomConstAccessorSP selectionAccessor;
110- if (d->selection) {
111- selectionAccessor = d->selection->projection()->createRandomConstAccessorNG(start.x(), start.y());
112- }
113-
114 KoColor lineColor(d->paintColor);
115
116- int x1 = start.x();
117- int y1 = start.y();
118- int x2 = end.x();
119- int y2 = end.y();
120+ int x1 = qFloor(start.x());
121+ int y1 = qFloor(start.y());
122+ int x2 = qFloor(end.x());
123+ int y2 = qFloor(end.y());
124
125+ KisRandomAccessorSP accessor = d->device->createRandomAccessorNG(x1, y1);
126+ KisRandomConstAccessorSP selectionAccessor;
127+ if (d->selection) {
128+ selectionAccessor = d->selection->projection()->createRandomConstAccessorNG(x1, y1);
129+ }
130
131 float grad, xd, yd;
132 float xgap, ygap, xend, yend, yf, xf;
133@@ -1854,21 +1849,20 @@ void KisPainter::drawWuLine(const QPointF & start, const QPointF & end)
134 // horizontal line
135 // line have to be paint from left to right
136 if (x1 > x2) {
137- float tmp;
138- tmp = x1; x1 = x2; x2 = tmp;
139- tmp = y1; y1 = y2; y2 = tmp;
140+ std::swap(x1, x2);
141+ std::swap(y1, y2);
142 xd = (x2 - x1);
143 yd = (y2 - y1);
144 }
145 grad = yd / xd;
146 // nearest X,Y interger coordinates
147- xend = static_cast<int>(x1 + 0.5f);
148+ xend = x1;
149 yend = y1 + grad * (xend - x1);
150
151 xgap = invertFrac(x1 + 0.5f);
152
153- ix1 = static_cast<int>(xend);
154- iy1 = static_cast<int>(yend);
155+ ix1 = x1;
156+ iy1 = qFloor(yend);
157
158 // calc the intensity of the other end point pixel pair.
159 brightness1 = invertFrac(yend) * xgap;
160@@ -1896,13 +1890,13 @@ void KisPainter::drawWuLine(const QPointF & start, const QPointF & end)
161 // calc first Y-intersection for main loop
162 yf = yend + grad;
163
164- xend = trunc(x2 + 0.5f);
165+ xend = x2;
166 yend = y2 + grad * (xend - x2);
167
168 xgap = invertFrac(x2 - 0.5f);
169
170- ix2 = static_cast<int>(xend);
171- iy2 = static_cast<int>(yend);
172+ ix2 = x2;
173+ iy2 = qFloor(yend);
174
175 brightness1 = invertFrac(yend) * xgap;
176 brightness2 = frac(yend) * xgap;
177@@ -1933,16 +1927,16 @@ void KisPainter::drawWuLine(const QPointF & start, const QPointF & end)
178 c1 = (int)(brightness1 * OPACITY_OPAQUE_U8);
179 c2 = (int)(brightness2 * OPACITY_OPAQUE_U8);
180
181- accessor->moveTo(x, int (yf));
182- if (selectionAccessor) selectionAccessor->moveTo(x, int (yf));
183+ accessor->moveTo(x, qFloor(yf));
184+ if (selectionAccessor) selectionAccessor->moveTo(x, qFloor(yf));
185
186 if (!selectionAccessor || *selectionAccessor->oldRawData() > SELECTION_THRESHOLD) {
187 lineColor.setOpacity(c1);
188 compositeOnePixel(accessor->rawData(), lineColor);
189 }
190
191- accessor->moveTo(x, int (yf) + 1);
192- if (selectionAccessor) selectionAccessor->moveTo(x, int (yf) + 1);
193+ accessor->moveTo(x, qFloor(yf) + 1);
194+ if (selectionAccessor) selectionAccessor->moveTo(x, qFloor(yf) + 1);
195
196 if (!selectionAccessor || *selectionAccessor->oldRawData() > SELECTION_THRESHOLD) {
197 lineColor.setOpacity(c2);
198@@ -1955,9 +1949,8 @@ void KisPainter::drawWuLine(const QPointF & start, const QPointF & end)
199 //vertical
200 // line have to be painted from left to right
201 if (y1 > y2) {
202- float tmp;
203- tmp = x1; x1 = x2; x2 = tmp;
204- tmp = y1; y1 = y2; y2 = tmp;
205+ std::swap(x1, x2);
206+ std::swap(y1, y2);
207 xd = (x2 - x1);
208 yd = (y2 - y1);
209 }
210@@ -1965,13 +1958,13 @@ void KisPainter::drawWuLine(const QPointF & start, const QPointF & end)
211 grad = xd / yd;
212
213 // nearest X,Y interger coordinates
214- yend = static_cast<int>(y1 + 0.5f);
215+ yend = y1;
216 xend = x1 + grad * (yend - y1);
217
218- ygap = invertFrac(y1 + 0.5f);
219+ ygap = y1;
220
221- ix1 = static_cast<int>(xend);
222- iy1 = static_cast<int>(yend);
223+ ix1 = qFloor(xend);
224+ iy1 = y1;
225
226 // calc the intensity of the other end point pixel pair.
227 brightness1 = invertFrac(xend) * ygap;
228@@ -1999,13 +1992,13 @@ void KisPainter::drawWuLine(const QPointF & start, const QPointF & end)
229 // calc first Y-intersection for main loop
230 xf = xend + grad;
231
232- yend = trunc(y2 + 0.5f);
233+ yend = y2;
234 xend = x2 + grad * (yend - y2);
235
236 ygap = invertFrac(y2 - 0.5f);
237
238- ix2 = static_cast<int>(xend);
239- iy2 = static_cast<int>(yend);
240+ ix2 = qFloor(xend);
241+ iy2 = y2;
242
243 brightness1 = invertFrac(xend) * ygap;
244 brightness2 = frac(xend) * ygap;
245@@ -2036,16 +2029,16 @@ void KisPainter::drawWuLine(const QPointF & start, const QPointF & end)
246 c1 = (int)(brightness1 * OPACITY_OPAQUE_U8);
247 c2 = (int)(brightness2 * OPACITY_OPAQUE_U8);
248
249- accessor->moveTo(int (xf), y);
250- if (selectionAccessor) selectionAccessor->moveTo(int (xf), y);
251+ accessor->moveTo(qFloor(xf), y);
252+ if (selectionAccessor) selectionAccessor->moveTo(qFloor(xf), y);
253
254 if (!selectionAccessor || *selectionAccessor->oldRawData() > SELECTION_THRESHOLD) {
255 lineColor.setOpacity(c1);
256 compositeOnePixel(accessor->rawData(), lineColor);
257 }
258
259- accessor->moveTo(int (xf) + 1, y);
260- if (selectionAccessor) selectionAccessor->moveTo(int (xf) + 1, y);
261+ accessor->moveTo(qFloor(xf) + 1, y);
262+ if (selectionAccessor) selectionAccessor->moveTo(qFloor(xf) + 1, y);
263
264 if (!selectionAccessor || *selectionAccessor->oldRawData() > SELECTION_THRESHOLD) {
265 lineColor.setOpacity(c2);
266@@ -2235,11 +2228,11 @@ void KisPainter::drawThickLine(const QPointF & start, const QPointF & end, int s
267 yfb = y0b + gradb;
268
269 for (x = ix1 + 1; x <= ix2 - 1; x++) {
270- fraca = yfa - int (yfa);
271+ fraca = yfa - qFloor(yfa);
272 b1a = 1 - fraca;
273 b2a = fraca;
274
275- fracb = yfb - int (yfb);
276+ fracb = yfb - qFloor(yfb);
277 b1b = 1 - fracb;
278 b2b = fracb;
279
280@@ -2247,8 +2240,8 @@ void KisPainter::drawThickLine(const QPointF & start, const QPointF & end, int s
281 opacity = ((x - ix1) / dstX) * c2.opacityF() + (1 - (x - ix1) / dstX) * c1.opacityF();
282 c3.setOpacity(opacity);
283
284- accessor->moveTo(x, (int)yfa);
285- if (selectionAccessor) selectionAccessor->moveTo(x, (int)yfa);
286+ accessor->moveTo(x, qFloor(yfa));
287+ if (selectionAccessor) selectionAccessor->moveTo(x, qFloor(yfa));
288
289 if (!selectionAccessor || *selectionAccessor->oldRawData() > SELECTION_THRESHOLD) {
290 qreal alpha = cs->opacityF(accessor->rawData());
291@@ -2259,8 +2252,8 @@ void KisPainter::drawThickLine(const QPointF & start, const QPointF & end, int s
292
293 // color first pixel of top line
294 if (!(startWidth == 1 && endWidth == 1)) {
295- accessor->moveTo(x, (int)yfb);
296- if (selectionAccessor) selectionAccessor->moveTo(x, (int)yfb);
297+ accessor->moveTo(x, qFloor(yfb));
298+ if (selectionAccessor) selectionAccessor->moveTo(x, qFloor(yfb));
299
300 if (!selectionAccessor || *selectionAccessor->oldRawData() > SELECTION_THRESHOLD) {
301 qreal alpha = cs->opacityF(accessor->rawData());
302@@ -2273,8 +2266,8 @@ void KisPainter::drawThickLine(const QPointF & start, const QPointF & end, int s
303 // color second pixel of bottom line
304 if (grada != 0 && grada != 1) { // if not flat or exact diagonal
305
306- accessor->moveTo(x, int (yfa) + 1);
307- if (selectionAccessor) selectionAccessor->moveTo(x, int (yfa) + 1);
308+ accessor->moveTo(x, qFloor(yfa) + 1);
309+ if (selectionAccessor) selectionAccessor->moveTo(x, qFloor(yfa) + 1);
310
311 if (!selectionAccessor || *selectionAccessor->oldRawData() > SELECTION_THRESHOLD) {
312 qreal alpha = cs->opacityF(accessor->rawData());
313@@ -2288,8 +2281,8 @@ void KisPainter::drawThickLine(const QPointF & start, const QPointF & end, int s
314 // color second pixel of top line
315 if (gradb != 0 && gradb != 1 && !(startWidth == 1 && endWidth == 1)) {
316
317- accessor->moveTo(x, int (yfb) + 1);
318- if (selectionAccessor) selectionAccessor->moveTo(x, int (yfb) + 1);
319+ accessor->moveTo(x, qFloor(yfb) + 1);
320+ if (selectionAccessor) selectionAccessor->moveTo(x, qFloor(yfb) + 1);
321
322 if (!selectionAccessor || *selectionAccessor->oldRawData() > SELECTION_THRESHOLD) {
323 qreal alpha = cs->opacityF(accessor->rawData());
324@@ -2303,7 +2296,7 @@ void KisPainter::drawThickLine(const QPointF & start, const QPointF & end, int s
325 // fill remaining pixels
326 if (!(startWidth == 1 && endWidth == 1)) {
327 if (yfa < yfb)
328- for (int i = yfa + 1; i <= yfb; i++) {
329+ for (int i = qFloor(yfa) + 1; i <= qFloor(yfb); i++) {
330
331 accessor->moveTo(x, i);
332 if (selectionAccessor) selectionAccessor->moveTo(x, i);
333@@ -2313,7 +2306,7 @@ void KisPainter::drawThickLine(const QPointF & start, const QPointF & end, int s
334 }
335 }
336 else
337- for (int i = yfa + 1; i >= yfb; i--) {
338+ for (int i = qFloor(yfa) + 1; i >= qFloor(yfb); i--) {
339
340 accessor->moveTo(x, i);
341 if (selectionAccessor) selectionAccessor->moveTo(x, i);
342@@ -2353,11 +2346,11 @@ void KisPainter::drawThickLine(const QPointF & start, const QPointF & end, int s
343 xfb = x0b + gradb;
344
345 for (y = iy1 + 1; y <= iy2 - 1; y++) {
346- fraca = xfa - int (xfa);
347+ fraca = xfa - qFloor(xfa);
348 b1a = 1 - fraca;
349 b2a = fraca;
350
351- fracb = xfb - int (xfb);
352+ fracb = xfb - qFloor(xfb);
353 b1b = 1 - fracb;
354 b2b = fracb;
355
356@@ -2365,8 +2358,8 @@ void KisPainter::drawThickLine(const QPointF & start, const QPointF & end, int s
357 opacity = ((y - iy1) / dstY) * c2.opacityF() + (1 - (y - iy1) / dstY) * c1.opacityF();
358 c3.setOpacity(opacity);
359
360- accessor->moveTo(int (xfa), y);
361- if (selectionAccessor) selectionAccessor->moveTo(int (xfa), y);
362+ accessor->moveTo(qFloor(xfa), y);
363+ if (selectionAccessor) selectionAccessor->moveTo(qFloor(xfa), y);
364
365 if (!selectionAccessor || *selectionAccessor->oldRawData() > SELECTION_THRESHOLD) {
366 qreal alpha = cs->opacityF(accessor->rawData());
367@@ -2378,8 +2371,8 @@ void KisPainter::drawThickLine(const QPointF & start, const QPointF & end, int s
368 // color first pixel of right line
369 if (!(startWidth == 1 && endWidth == 1)) {
370
371- accessor->moveTo(int(xfb), y);
372- if (selectionAccessor) selectionAccessor->moveTo(int(xfb), y);
373+ accessor->moveTo(qFloor(xfb), y);
374+ if (selectionAccessor) selectionAccessor->moveTo(qFloor(xfb), y);
375
376 if (!selectionAccessor || *selectionAccessor->oldRawData() > SELECTION_THRESHOLD) {
377 qreal alpha = cs->opacityF(accessor->rawData());
378@@ -2392,8 +2385,8 @@ void KisPainter::drawThickLine(const QPointF & start, const QPointF & end, int s
379 // color second pixel of left line
380 if (grada != 0 && grada != 1) { // if not flat or exact diagonal
381
382- accessor->moveTo(int(xfa) + 1, y);
383- if (selectionAccessor) selectionAccessor->moveTo(int(xfa) + 1, y);
384+ accessor->moveTo(qFloor(xfa) + 1, y);
385+ if (selectionAccessor) selectionAccessor->moveTo(qFloor(xfa) + 1, y);
386
387 if (!selectionAccessor || *selectionAccessor->oldRawData() > SELECTION_THRESHOLD) {
388 qreal alpha = cs->opacityF(accessor->rawData());
389@@ -2407,8 +2400,8 @@ void KisPainter::drawThickLine(const QPointF & start, const QPointF & end, int s
390 // color second pixel of right line
391 if (gradb != 0 && gradb != 1 && !(startWidth == 1 && endWidth == 1)) {
392
393- accessor->moveTo(int(xfb) + 1, y);
394- if (selectionAccessor) selectionAccessor->moveTo(int(xfb) + 1, y);
395+ accessor->moveTo(qFloor(xfb) + 1, y);
396+ if (selectionAccessor) selectionAccessor->moveTo(qFloor(xfb) + 1, y);
397
398 if (!selectionAccessor || *selectionAccessor->oldRawData() > SELECTION_THRESHOLD) {
399 qreal alpha = cs->opacityF(accessor->rawData());
400@@ -2421,7 +2414,7 @@ void KisPainter::drawThickLine(const QPointF & start, const QPointF & end, int s
401 // fill remaining pixels between current xfa,xfb
402 if (!(startWidth == 1 && endWidth == 1)) {
403 if (xfa < xfb)
404- for (int i = (int) xfa + 1; i <= (int) xfb; i++) {
405+ for (int i = qFloor(xfa) + 1; i <= qFloor(xfb); i++) {
406
407 accessor->moveTo(i, y);
408 if (selectionAccessor) selectionAccessor->moveTo(i, y);
409@@ -2431,7 +2424,7 @@ void KisPainter::drawThickLine(const QPointF & start, const QPointF & end, int s
410 }
411 }
412 else
413- for (int i = (int) xfb; i <= (int) xfa + 1; i++) {
414+ for (int i = qFloor(xfb); i <= qFloor(xfa) + 1; i++) {
415
416 accessor->moveTo(i, y);
417 if (selectionAccessor) selectionAccessor->moveTo(i, y);

Diff Detail

Repository
R37 Krita
Lint
Lint Skipped
Unit
Unit Tests Skipped
alvinhochun created this revision.Nov 19 2017, 4:41 PM
dkazakov accepted this revision.Nov 20 2017, 1:44 PM
dkazakov added a subscriber: dkazakov.

Hi, @alvinhochun!

The patch looks nice, please push! :)

This revision is now accepted and ready to land.Nov 20 2017, 1:44 PM
This revision was automatically updated to reflect the committed changes.