Re: Swing Dev>[9] Review Request JDK-8163160 [PIT][TEST_BUG] Some issues in java/awt/image/multiresolution/MultiResolutionIcon/IconTest.java

2016-08-18 Thread Rajeev Chamyal
Hello Yuri,

Can you please review below webrev.
Webrev: http://cr.openjdk.java.net/~rchamyal/8163160/webrev.00/

Regards,
Rajeev Chamyal

-Original Message-
From: Rajeev Chamyal 
Sent: 16 August 2016 18:45
To: Sergey Bylokhov; Alexander Scherbatiy; swing-dev@openjdk.java.net
Subject: Re:  Swing Dev>[9] Review Request JDK-8163160 
[PIT][TEST_BUG] Some issues in 
java/awt/image/multiresolution/MultiResolutionIcon/IconTest.java

Hello Sergey,

Thanks for the review. In the bug its reported that border of button is grey. 
Instead of button border icon border should be checked. I have updated test 
instructions for this.

Regards,
Rajeev Chamyal

-Original Message-
From: Sergey Bylokhov 
Sent: 16 August 2016 18:28
To: Rajeev Chamyal; Alexander Scherbatiy; swing-dev@openjdk.java.net
Subject: Re: Swing Dev>[9] Review Request JDK-8163160 [PIT][TEST_BUG] Some 
issues in java/awt/image/multiresolution/MultiResolutionIcon/IconTest.java

On 16.08.16 15:57, Sergey Bylokhov wrote:
> The changes looks fine, there are a notice, is it a known issues?

there are a notice about different background of the buttons.

>
> On 16.08.16 15:01, Rajeev Chamyal wrote:
>> Hello All,
>>
>>
>>
>> Please review the following webrev.
>>
>>
>>
>> Webrev: http://cr.openjdk.java.net/~rchamyal/8163160/webrev.00/
>>
>> Bug : https://bugs.openjdk.java.net/browse/JDK-8163160
>>
>> Issue : manual tag was missing in test.
>>
>>
>>
>> Regards,
>>
>> Rajeev Chamyal
>>
>>
>>
>
>


-- 
Best regards, Sergey.


Re: [9] Review request for 8156217 Selected text is shifted on HiDPI display

2016-08-18 Thread Philip Race

OK .. I do not know enough about how modelToView is used by Swing
to know what is really needed here. Someone with a bit more Swing
background needs to chime in. I was encouraged that the *API* surface
of the changes was much smaller than it had seemed from the webrev
but maybe that is because you did not include everything. For example
although they are just subclassing the method overrides in PasswordView
since that is a public class would become part of the spec .. would they 
not  ?

Just like the "int" counterparts today :-
https://docs.oracle.com/javase/8/docs/api/javax/swing/text/PasswordView.html

Put another way I was looking for what the content of the CCC would be.

-phil.


On 8/15/16, 11:48 AM, Alexander Scherbatiy wrote:

On 15/08/16 21:43, Phil Race wrote:

Why is the caret support added in here ? Same for the modelToView
That will just hold this up as the reasoning behind needing those 
changes is not something
I have yet been able to convince myself about - even after reading 
your last email.


  The main change for the Caret public API (methods 
Caret.getMagicCaretPosition2D()/setMagicCaretPosition2D(Point2D p)) is 
not included in the current fix. I only moved the new methods 
JTextComponent.modelToView2D(int pos)/viewToModel2D(Point2D pt) from 
the fix for the Caret to this fix. These methods are used not only for 
caret but in other cases like mouse handling, text dragging and others.


Thanks,
Alexandr.




-phil.



On 08/15/2016 04:13 AM, Alexandr Scherbatiy wrote:


  Hello,

  Could you review the updated fix?
webrev which contains only change in public API: 
http://cr.openjdk.java.net/~alexsch/8156217/webrev.05/public-api
webrev with contains all changes: 
http://cr.openjdk.java.net/~alexsch/8156217/webrev.05/all


  - methods with int coordinates are deprecated
  - public isUseFloatingPointAPI()/setUseFloatingPointAPI() methods 
are added to the PlainView and WrappedPlainView classes
  - JTextComponent.modelToView2D(int pos)/viewToModel2D(Point2D pt) 
public methods from fix JDK-8163124 Add floating point API support 
to javax.swing.text.Caret

are added
  - some @implSpec descriptions are removed from the new text 
drawing methods with floating point arguments
  - Built-in L&Fs are updated to use floating point API in standard 
Java text components


  Thanks,
  Alexandr.

On 7/28/2016 5:38 PM, Alexandr Scherbatiy wrote:


See comments inline.

On 7/26/2016 11:57 PM, Phil Race wrote:
I have a lot of doubts about this as well as trouble getting my 
head around all of it.


Given that apps need to 'buy in' to the floating point I am not 
sure what we are gaining

but I need to make sure I understand the problem.

It affects only the  methods that the 3rd party code can over-ride
in subclasses and that are called by the JDK internal code.

There are just two protected methods that matter :-
PlainView.drawSelectedText(..)
and
PlainView.drawUnselectedText(..)

The hidpi precison matters since they are drawing a sub-range of 
the text.

Is there any other method that matters / is used in this way ?
  I have found the following methods which relate to text drawing, 
can be overridden and could have floating point coordinates:


javax.swing.text.PlainView.drawLine(...)
javax.swing.text.PlainView.lineToRect(...)
javax.swing.text.PasswordView.drawEchoCharacter(...)

javax.swing.plaf.TextUI.modelToView(...)
javax.swing.plaf.TextUI.viewToModel(...)
javax.swing.plaf.TextUI.getToolTipText(...)

There is also a method which relates to a caret position in a text:
  javax.swing.text.DefaultCaret.setMagicCaretPosition(Point p)
This requires additional investigation because DefaultCaret extends 
Rectangle and so its coordinates can't be float.




Since 3rd party code is not over-riding these they will get the JDK
super-class version, thus losing any customisation they might have 
done

in the no-longer-called int version.

Assuming that is correct, what customisation would be lost and how 
much does it matter?


The example is javax.swing.text.PasswordView class which overrides 
drawSelectedText(...)/drawUnselectedText(...) methods and draws 
echo chars instead of text.

The similar can be done in a custom component:

 public class CustomPasswordField extends FieldView {

 @Override
 protected int drawSelectedText(Graphics g, int x, int y, int 
p0, int p1) throws BadLocationException {

 // draw echo chars
 }
 }


Switching to support new methods with floating point coordinates 
will lead that real text will be shown for old applications in 
password fields.


My prefernce is to deprecate the int versions and always call the 
float versions

rather than the opt-in approach.

Actually my real preference would be to come up with something 
that does

not involve drawing the text in chunks like this.

ie Swing should use AttributedCharacterIterator ..  it looks like 
the code to

do this might already be there !

  106 private float drawElement(

Re: [9] Review request for 8164032 JViewport backing store image is not scaled on HiDPI display

2016-08-18 Thread Alexandr Scherbatiy


Hello,

Could you review the updated fix:
  http://cr.openjdk.java.net/~alexsch/8164032/webrev.02

  The backing store image is recreated for the case where scaled sizes 
are changed.


Thanks,
Alexandr.

On 8/17/2016 5:05 PM, Sergey Bylokhov wrote:

On 17.08.16 14:02, Alexandr Scherbatiy wrote:


Hello,

Could you review the updated fix:
  http://cr.openjdk.java.net/~alexsch/8164032/webrev.01

The generic transform is used for the backing store image scaling.


As far as I understand the current fix will not work if the scale was 
changed(the JViewport will use old buffer)?


It seems that the new local AbstractMultiResolutionImage contain only 
one image variant inside and looks quite similar to the VolatileImage, 
probably we can change this cache from BufferedImage to VolatileImage? 
In this case the scale of the VI will be the same as "g":

g.getDeviceConfiguration().createCompatibleVolatileImage()



On 8/15/2016 4:58 PM, Sergey Bylokhov wrote:

Hi, Alexandr.
I doubt that getScaleX/getScaleY can be used here because the scale
can be generic(translate+rotate+scale). How this cache will work if
transform will be changed after we save "backingStoreImage"?

On 15.08.16 16:16, Alexandr Scherbatiy wrote:


Hello,

Could you review the fix:
  bug: https://bugs.openjdk.java.net/browse/JDK-8164032
  webrev: http://cr.openjdk.java.net/~alexsch/8164032/webrev.00

  The fix scales the JViewport backing store image when graphics
transform is not identity.

 Thanks,
 Alexandr.