I suppose that there could be a 4th bug about the fact that the trimming/refining code in the native TransformHelper is
based on an inverse transform and subject to bit error, but it's not easy to figure out how to refine it further. Note
that it could also cause us to omit a row of pixels in some cases if the rounding went the other way. Refining it
further could be easier than the 2nd two solutions, but not really noticeable if we just get do the more conservative
calculations on the bounds equations in the setup code...
...jim
On 10/28/16 2:33 PM, Jim Graham wrote:
Hi Alexandr,
That code is coming up with maximum extents for the operation, not the exact
edges that will be rendered. It passes
them down to the native TransformHelper method which refines them using the
exact calculations it will use to render the
pixels.
Admittedly, the calculations you point out below in the DrawImage pipeline
would more accurately note that the
right/bottom edges fall on a .5 pixel location and shouldn't be included, but I
didn't want to make that decision using
one set of equations while the actual rendering uses different math down below.
Note that if you had a rotation then there are lots of unused pixels on each
scanline which are computed and refined by
the native code. Those calculations would also be performing the
"included/excluded" calculations using the actual
rendering equations rather than these higher level boundary equations so even if we
switched to "ceil(N - 0.5)"
calculations up here for the bounding box to cure the scaled drawImage we'd
still potentially have overshoot due to the
fixed point/floating point bit errors on those rotated edges.
With a general transform it would be difficult to detect the occasional extra
pixel being rendered due to bit errors,
but it is much easier with these scale-only transforms.
One change we could make would be to do the ceil(N-0.5) calculation in the bounds setup
code which would "fix" the scale
only case.
Another might be to fix the scale pipeline to handle arbitrary formats - we could write a
related "ScaleHelper" native
method that would do the same thing that TransformHelper does. TransformHelper
takes a transform loop that transforms
pixels to ARGB, and a separate Blit or MaskBlit that blends ARGB pixels into
the destination and it runs the first loop
into a temporary local buffer on the C call stack and the second loop to blend
the pixels into the dest. A ScaleHelper
loop would do the same, but with a "Scale(XXX to ARGB)" loop for the first half.
Finally, we could also add more ScaleBlit loops for more source/dest pairs so
these scales would happen more directly.
Unfortunately, many of the missing cases would require creating scale loops
that also do blending and the only macros we
currently have for direct src->dest scaling are opaque-to-opaque pixel store
operations.
The first solution is simpler and easier to add, the second one would help with
all scales that fall outside our loops.
The last solution would arguably provide the highest performance for any
source/dest case that we care about (and with
the advent of HiDPI we now care about a lot more cases).
I'd file 3 bugs:
- Scaled and transformed drawImage can overshoot with some scales (1.5)
- Add more ScaledBlit graphics primitive instances for common HiDPI cases
- Create a general purpose Scale helper method along the lines of
TransformHelper
The first could be fixed soon with the proposed change you talk about below...
...jim
On 10/28/16 6:40 AM, Alexandr Scherbatiy wrote:
On 10/11/2016 10:13 PM, Sergey Bylokhov wrote:
On 11.10.16 21:54, Jim Graham wrote:
Additionally, for AA rendering, there is no such thing as
"setClip(someShape)" and "fill(sameShape)" being identical no matter how
much we predict which rules they may want because clips are inherently
non-AA and so the fuzzy pixels along the boundary of an AA shape will be
randomly clipped or included.
When all is said and done I feel (not very strongly) it would be best to
have clipping remain unaffected by the biasing of STROKE hints, but part
of that is driven by the fact that I think it can be fixed with a couple
of lines of code in SG2D/LoopPipe...
I guess a bottom line is that it is require an additional investigation. I am
not exactly sure, but my expectation is
that fillRect(x,y,w,h)/drawImage(x,y,w,h) + setClip(x,y,w,h) should work in a
similar way(covers the same pixels). And
the question should this behavior be exactly the same as fill(RectShape) +
setClip(RectShape) is unclear (I am not
sure is it a critical issue or not)
Right now I tried to fix overlapping, which produce visible artifacts and were
considered as a bugs. The next issue
which I would like to fix is a overlapping of drawImage().
I just created a small sample [1] which fills a rectangle (x, y, w, y),
creates an image with size (w, h) and draws
the image into the area (x, y, w, h).
With the floating point scale like 1.5 the image does not exactly fits the
area filled by the rectangle (see image [2]
generated by the code [1]).
This is because the Graphics.drawImage(...) calls DrawImage.renderImageXform()
which uses floor and ceil methods for
the region corner rounding:
final int dx1 = Math.max((int) Math.floor(ddx1), clip.getLoX());
final int dy1 = Math.max((int) Math.floor(ddy1), clip.getLoY());
final int dx2 = Math.min((int) Math.ceil(ddx2), clip.getHiX());
final int dy2 = Math.min((int) Math.ceil(ddy2), clip.getHiY());
But the Graphics.fillRect() falls down to the code which just cast the
transformed coordinates to int.
Why the floor/ceil methods are used for the image region rounding?
Is it possible to change this to fit the rule for the filled rectangles
rounding?
[1] http://cr.openjdk.java.net/~alexsch/fpapi/code/FillRectAndImageTest.java
[2] http://cr.openjdk.java.net/~alexsch/fpapi/screenshots/rect-and-image.png
Thanks,
Alexandr.
