<Swing Dev> [7] Review request for 7123767 Wrong tooltip location in Multi-Monitor configurations

[Pavel Porvatov]

Hi Vladislav,

I have several comments about the fix:
1. getDrawingRect: now localInsets should be requested only if 
localRect.contains(toFind) is true
2.
+                // Something's really wrong here - use the screen that our
+                // Component belongs to
+                if (workingBounds == null)
+                    workingBounds = 
insideComponent.getGraphicsConfiguration().getBounds();
a. You should always use {...} for if construction
b. It seems we should invoke adjustInsets there. To reduce the code I 
think you can rename getDrawingRect into getGraphicsConfiguration and 
return GraphicsConfiguration there. And only when GraphicsConfiguration 
is found calculate adjusted bounds...
3. getDrawingRect comment should have two ")" at the end

Abou the test:
1. You should use Toolkit.realSync after setVisible
2. I run the test on jdk1.8.0b51 and it passes. Looks strange...

Regards, Pavel
> Hi Pavel, all,
>
> please find new webrev here: 
> http://cr.openjdk.java.net/~vkarnauk/7123767/webrev.02/
> Please see my comments inline.
>
> Regards,
> - Vlad
>
> On 8/17/2012 9:10 PM, Pavel Porvatov wrote:
>> Hi Vladislav,
>>
>> I have several comments about the fix:
>>
>> 1. javax.swing.ToolTipManager#showTipWindow: I think toFind variable 
>> should be initialized in if(preferredLocation != null) { ....} else 
>> {...} block. It looks more logical and code will be more compact 
>> (adjustedPreferredLocation can be removed at all, I think)
> Agreed. I've re-worked this piece.
>
>>
>> 2. Could you please explain why you are using union of all rectangles 
>> in the getDrawingRect method? In case two monitors are aligned on a 
>> diagonal totalRect will contain areas that are not covered by screen 
>> devices and popups can be placed incorrectly.
> Agreed again. I've re-designed getDrawingRect() function and placed 
> additional condition into showTipWindow() function; now every 
> situation should be handled correctly.
>
> Automated test was added, please see the webrev.
>
> Regards,
> - Vlad
>
>>
>> Regards, Pavel
>>
>>> Hello,
>>>
>>> please review the fix for 7123767: Wrong tooltip location in 
>>> Multi-Monitor configurations
>>>
>>> jdk7 webrev: http://cr.openjdk.java.net/~vkarnauk/7123767/webrev.00/
>>> test source: http://cr.openjdk.java.net/~vkarnauk/7123767/test/
>>> bug description: 
>>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7123767
>>>
>>> This is also a customer escalated issue in jdk6.
>>>
>>> On multi-monitor configurations Component.getLocationOnScreen() may 
>>> return negative values. Additionally, ToolTipManager.showTipWindow() 
>>> method calculates tip window location from Component' 
>>> getLocationOnScreen() return data and draws tip window using the 
>>> same GraphicsConfiguration instance as component's top-left corner, 
>>> which again can be of negative value. When component is stretched 
>>> across multiple screen devices, this may lead to a situation when 
>>> it's tooltip is drawn on wrong monitor.
>>>
>>> To fix this we should count all GraphicsConfiguration instances and 
>>> determine correct screen device to draw on. We should also take into 
>>> account that:
>>> 1) tip window preferred location could be set
>>> 2) preferred location could be greater than total virtual bounds of 
>>> all monitors. To make tip window visible in such case we move it to 
>>> the corresponding corner of total virtual bounds
>>> 3) exact order of monitors (left-to-right and top-to-bottom) is unknown
>>>
>>> I run jtreg against ToolTipManager tests and all tests passed (I run 
>>> it several times with different monitor configurations). I also 
>>> tested the fix with custom test program (please see the link 
>>> provided) with 4 possible monitor configurations (left-to-right, 
>>> right-to-left, top-to-bottom and bottom-to-top) and saw no issues.
>>>
>>> I added no new automated test(s) because this at least would require 
>>> multiple monitors to run.
>>>
>>> Regards,
>>> - Vlad
>>
>
>