On 29.11.16 20:46, Alexandr Scherbatiy wrote:
The result of running SwingMark 2 with the following JDK is: 1. without the fix 1st test run [1]: 92373 2nd test run [2]: 92156average: (92373 + 92156) / 2 = 92264.5 2. paintDoubleBufferedImp() method is always used 1st test run [3]: 92476 // (92476 - 92264.5) / 92264.5 / 100 = 0.000023% 2nd test run [4]: 90800 // (90800 - 92264.5) / 92264.5 / 100 = -0.000159% 3.paintDoubleBufferedFPScales () method is always used 1st test run [5]: 91053 // (91053 - 92264.5) / 92264.5 / 100 = -0.000131% 2nd test run [6]: 92900 // (92900 - 92264.5) / 92264.5 / 100 = 0.000069%
So it seems we can simplify the this codepath and always use the new method? Do we have some arguments against it?
[1] http://cr.openjdk.java.net/~alexsch/8162350/swingmark2/repaint-manager-fp-scale-base_00.txt [2] http://cr.openjdk.java.net/~alexsch/8162350/swingmark2/repaint-manager-fp-scale-base_01.txt [3] http://cr.openjdk.java.net/~alexsch/8162350/swingmark2/repaint-manager-fp-scale-int_00.txt [4]http://cr.openjdk.java.net/~alexsch/8162350/swingmark2/repaint-manager-fp-scale-int_01.txt [5] http://cr.openjdk.java.net/~alexsch/8162350/swingmark2/repaint-manager-fp-scale-fp_00.txt [6] http://cr.openjdk.java.net/~alexsch/8162350/swingmark2/repaint-manager-fp-scale-fp_01.txtOn 21.11.16 16:59, Alexandr Scherbatiy wrote:Hello, Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8162350/webrev.04 - isFloatingPointScale(AffineTransform) is moved from the SunGraphics2D to the SwingUtilities2 class. Thanks, Alexandr. On 11/18/2016 11:23 PM, Jim Graham wrote:Hi ALexandr, This looks great. BTW, when I suggested moving the FPscale test into SG2D I was suggesting that to avoid having to copy the transform out of it via getTransform(), but you've found a different solution to that issue (i.e. the new getTransform(g) method) so it no longer matters where that utility static function is located. You can move it back to one of the Swing classes. In terms of the logic of choosing which repaint function to use, it looks like you use the old-style function if the scales don't match, but won't that cause rendering anomalies? The new code is still an improvement for the standard HiDPI case, and I'm guessing that mismatched scales probably never tends to happen, but we might want to flag it for further investigation. +1 relative to whether you want to move the FPscale test back out of SG2D or not... ...jim On 11/18/16 1:44 AM, Alexandr Scherbatiy wrote:Thank you. I see that using the integer device-pixel translations preserves the component painting in the same way for floating point scales. Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8162350/webrev.03 - translation adjustment is removed - Region.clipRound() is used for pixels coordinates rounding. Thanks, Alexandr. On 11/16/2016 1:52 AM, Jim Graham wrote:Let me clarify something... On 11/15/16 2:49 AM, Alexandr Scherbatiy wrote:Let's consider the following use case: scale = 1.5 A component calls fillRect(1, 1, 1, 1). This is (1.5, 1.5, 3.0, 3.0) in the device space which fills (1, 1, 3, 3) and covers 2x2 pixelsAgreed.Now the area (1, 1, 1, 1) needs to be repainted create a backbuffer translate(-1, -1) // move the top left corner of the area to the zero point draw the component into the backbuffer: fillRect(1, 1, 1, 1) -> after translation fillRect(0, 0, 1, 1) -> after scaling (0.0, 0.0, 1.5, 1.5 ) in the device space which fills (0, 0, 1, 1) and covers 1x1 pixelsIf you did g.setTransform(identity), g.translate(-1, -1), (then restore the scale) then the analysis is as follows: g.setTransform(identity) => [1 0 0] [0 1 0] g.translate(-1, -1) => [1 0 -1] [0 1 -1] g.scale(1.5, 1.5) => [1.5 0 -1] [0 1.5 -1] g.fillRect(1, 1, 1, 1) => coordinates are (1.5-1, 1.5-1, 3-1, 3-1) => (.5, .5, 2, 2) => fills (0, 0, 2, 2) => which covers 2x2 pixels If you did g.translate(-1, -1) on the scaled transform then the analysis is as follows: g.transform is [1.5 0 0] [0 1.5 0] g.translate(-1, -1) is [1.5 0 -1.5] [0 1.5 -1.5] g.fillRect(1, 1, 1, 1) => coordinates are (1.5-1.5, 1.5-1.5, 3-1.5, 3-1.5) => (0, 0, 1.5, 1.5) => fill (0, 0, 1, 1) => covers 1x1 pixels The second operation is what you are describing above and that would be an inappropriate way to perform damage repair because you used a scaled translation which did not result in an integer coordinate translation. Please re-read my previous analysis that shows what happens when you use integer device-pixel translations which are translations that happen using integers on a non-scaled transform. Note that you can add a scale *AFTER* you apply the integer device pixel translation and it will not affect the integer-ness of the translation. You can see above that the difference in how the translate command is issues affects where the translation components of the matrix end up being -1,-1 or -1.5,-1.5... ...jim
-- Best regards, Sergey.
