Fix the rounding error of negative coordinates in wrap around mode for some operations, e.g. colour picker and fill tool.
Details
Details
- Reviewers
dkazakov - Group Reviewers
Krita - Commits
- R37:58c3af947fbf: Fix rounding in wrap around mode for some operations
Diff Detail
Diff Detail
- Repository
- R37 Krita
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Comment Actions
P.S. there is a QPoint KisImage::documentToIntPixel( const QRectF &) overload which I haven't touched. With a quick search it seems to be unused, and I have no clue if it needs fixing or not.
Comment Actions
P.S. there is a `QPoint KisImage::documentToIntPixel( const QRectF &)` overload which I haven't touched. With a quick search it seems to be unused, and I have no clue if it needs fixing or not.
If it's not used, it should be removed.
Comment Actions
Please change the rounding to 'nearest' and push :)
| libs/image/kis_image.cc | ||
|---|---|---|
| 944 | It should return return pixelCoord.toPoint() instead, that is "round to nearest". Otherwise, the coordinate will not coincide with the results returned by KisCoordinatesConverter. | |
| libs/image/kis_image.cc | ||
|---|---|---|
| 944 | Really though? I just tried this and the colour picker is offset by half a pixel, for both positive and negative coords. | |
| libs/image/kis_image.cc | ||
|---|---|---|
| 944 | Then it looks like there is something wrong. Either is color picker or in KisCoordinatesConverter. We need to discuss that on IRC some time this week | |
| libs/image/kis_image.cc | ||
|---|---|---|
| 944 | I think it's not the color picker; the fill tool and the contiguous selection tool also use the same way to transform the coordinates.from the KoPointerEvent. The source point of the KoPointerEvent seems to come from KisToolProxy::widgetToDocument which in turn appears to be from KisCoordinatesConverter. Seeing that QPointF KisImage::pixelToDocument(const QPoint &pixelCoord) const returns QPointF((pixelCoord.x() + 0.5) / xRes(), (pixelCoord.y() + 0.5) / yRes()), using qFloor here seems to be the right choice, and it does work correctly, so it would seem to coincide with KisCoordinatesConverter, wouldn't it? | |
Comment Actions
As discussed on IRC, the floor rounding is correct, but the related functions are now renamed for clarity