Hi, Prasanta. 

I have one additional suggestion. I think we need to check the whole related 
sequence of bugs 



0) JDK-8001470 - The bug which is unrela6ed to the current bug, but it is 
introduce the test from which we start the step (1) 


1) Initial bug was reproduced on Solaris 11 only : 
https://bugs.openjdk.java.net/browse/JDK-8130892 
As a fix the check for "uninitialized" state was changed, but it was caused 
next regression 



2) https://bugs.openjdk.java.net/browse/JDK-8160246 , it also reproduced only 
on Solaris and this is the fix where we added the "rootViewInitialised" flag, 
but it cause next regression 


2) Current bug https://bugs.openjdk.java.net/browse/JDK-8226513 



And you already found by the test that it dose not take into account the fact 
that the GConfig may be different when you calculate the size for invisible 
component, and the size after it is became visible, probably ithe fix itself 
does not take into account this? 


It is also quite interesting to check the root cause of the JDK-8130892 : 
https://bugs.openjdk.java.net/browse/JDK-8130892?focusedCommentId=13822701&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13822701
 


I doubt that negative size is a correct value for the component. 


----- [email protected] wrote: 
> 
OK +1 from me ! 
> 
> -phil. 
> 
> 
> On 8/8/19 2:23 AM, Prasanta Sadhukhan wrote: 
> 




> 
> On 07-Aug-19 9:42 PM, Phil Race wrote: 
> 

I have a minor quibble that "rootViewInitialized" is no longer a very 
appropriate variable name, 
> since it is not just about initialization. 
> Can we rename it to "rootViewNeedsLayout" ? 
> 
> Renamed. 
> 


> Also because the windows 10 issue has a different cause, potentially this 
> test can be adjusted 
> to allow some tolerance to tell the difference between 
> "not re-layed out at all", and "I'm a few pixels off in my expectations". 
> ie the test should be about "did I relayout?", not "is my preferred size 
> exactly the actual size?". 
> 
> Could this be a hi-dpi issue ? Are you running at 96 dpi or something else 
> when this fails ? 
> 
> 

Yes, it is. I have modified the jtreg command line to test in uiScale 1. It now 
pass in windows and others and fails without the fix. 

http://cr.openjdk.java.net/~psadhukhan/8226513/webrev.02/ Regards 
> Prasanta 
> 


> -phil. 
> 
> 
> 
> On 8/7/19 2:04 AM, Prasanta Sadhukhan wrote: 
> 



I confirm that the test pass on linux and mac with the fix and fail without the 
fix. Only on windows, it fails. 
> 

But I also see that the failure is not because of the fix. The "problem" was 
there even before the fix, for example with jdk13 (fails with jdk11, jdk12 too 
) 
> 

jdk13-b24/bin/java JEditorPaneLayoutTest 
> Exception in thread "main" java.lang.RuntimeException: Wrong size 
> java.awt.Dimension[width=183,height=10] expected 
> java.awt.Dimension[width=177,height=44] 
> at JEditorPaneLayoutTest.main(JEditorPaneLayoutTest.java:84) 
> 

which got somewhat improved by JDK-8217731 

jdk13-b25/bin/java JEditorPaneLayoutTest 
> Exception in thread "main" java.lang.RuntimeException: Wrong size 
> java.awt.Dimension[width=181,height=10] expected 
> java.awt.Dimension[width=180,height=44] 
> at JEditorPaneLayoutTest.main(JEditorPaneLayoutTest.java:84) 

So, the 1 pixel difference in width was there even before this fix and also 
this fix fixes the height of the basic text component and the unit tests of JBS 
works ok, so to me this fix looks ok to me. 

+1 for the fix from me and 
> 

I have added the test to problem list only for windows to figure out the 1 
pixel difference in width which seems to be because of layouting in windows. 
> 

http://cr.openjdk.java.net/~psadhukhan/8226513/webrev.01/ Regards 
> Prasanta 
> 
> On 22-Jul-19 4:01 PM, Prasanta Sadhukhan wrote: 
> 

Hi Semyon, 
> 
> Although the JBS testcase is passing with your change, your testcase is 
> failing even with the fix for me in windows10 
> 
> java.lang.RuntimeException: Wrong size 
> java.awt.Dimension[width=181,height=44] expected 
> java.awt.Dimension[width=180,height=44] 
> at JEditorPaneLayoutTest.main(JEditorPaneLayoutTest.java:84) 
> 
> Regards 
> 
> Prasanta 
> 
> On 17-Jul-19 1:33 AM, [email protected] wrote: 
> 

bug: https://bugs.openjdk.java.net/browse/JDK-8226513 
> 
> webrev: http://cr.openjdk.java.net/~ssadetsky/8226513/webrev.00/ 
> 
> The fix adds resetting the root view initialization flag when the text 
> component underling document is changed and also removes the check for the 
> zero component size for the root view initialization to correct the resulting 
> preferred component size in some scenarios when the root view need to be 
> initially layouted. 
> 
> --Semyon 
> 
> 
> 
> 
>

Reply via email to