[OpenJDK 2D-Dev] [9] RFR: JDK-8138749, , Revisited: PrinterJob.printDialog() does not support multi-mon, always displayed on primary

[prasanta sadhukhan]

Hi Phil,

On 2/3/2016 6:05 AM, Phil Race wrote:
> The RasterPrinterJob changes are now OK however
>
> - I am not sure I understand how removing the check in ServiceUI
> will address the issue about the dialog needing to be entirely on-screen.
> It reads to me as if now you allow that in preference to having it 
> centred.
> But I am not sure the old code would have worked in all cases either
> since centering on a very small owner window in the corner of the screen
> would result in the same problem.
Since this issue was about showing the dialog on primary vs secondary, 
can we address this issue of showing the dialog entirely on-screen in 
separate bug id (since this issue is present in primary screen too if 
dialog is bigger than top-level window)?
The present change in ServiceUI address the issue of showing the dialog 
on secondary monitor if dialog is bigger than top-level.
> Probably what is needed is some code that moves it just enough to fit.
> ie downwards if it is off the top, left if it is off the right and so 
> forth ?
>
> - The two tests do not share the same bug ID. I expected they
> should now do so .. and I was further supposing you would merge
> the two tests into one.
I will change the bug id of 2nd test. Is it ok if I keep these 2 tests 
separate to make it easier?

Regards
Prasanta
>
> -phil.
>
> On 01/29/2016 01:34 AM, prasanta sadhukhan wrote:
>> Hi Phil,
>>
>> On 1/28/2016 8:22 PM, Philip Race wrote:
>>> "in consistent"
>>>
>>> On 1/21/16, 11:20 PM, prasanta sadhukhan wrote:
>>>> Hi Phil,
>>>>
>>>> I checked with NATIVE print dialog and with other app print dialog, 
>>>> they are positioning the dialog a little ahead of top-left corner 
>>>> (not at 1/3rd of the window as we are doing now)
>>>> so I did the same and positioned the dialog in consistent with our 
>>>> NATIVE print dialog.
>>>
>>> You mean consistently. in consistent means the opposite
>>> Please adjust that in the code comment.
>>>
>>> 50,50 is probably OK so long as you verified this is approximately
>>> what happens on most platforms, not just windows 8 or similar.
>>> I don't think you will find complete consistency because apps may
>>> control it and use different dialogs even on the same platform.
>>> On hidpi screens this may be closer to the top-left than you intend
>>> but I expect it is fine.
>>>
>> Tested on windows, linux and it works ok. We showed the dialog at 
>> x+50,y+50 where x,y is the coordinate of the active window.
>> Do not have local mac system to test multimonitor setup.
>>> The one main thing that you should verify is what happens in the case
>>> a native app top-level window is smaller than the print dialog and 
>>> is positioned
>>> at the bottom right (of the right-most display). Does the native app 
>>> ensure
>>> that the dialog is wholly on-screen ? I don't think your code will 
>>> do that.
>>> And Safari on Mac at least seems to do that. I guess that most apps try
>>> their best to keep the dialog wholly on-screen.
>>>
>> Tested this case where window bounds is smaller than print dialog
>> but because of this check in ServiceUI.java
>>
>> // if portion of dialog is not within the gc boundary
>>         if (!gcBounds.contains(dlgBounds)) {
>>             // put in the center relative to parent frame/dialog
>>             dialog.setLocationRelativeTo(owner);
>>         }
>>
>> the dialog is shown in the primary window.
>> I saw we did not have this check for pageDialog(attr) where the 
>> dialog is shown in secondary monitor despite gcBounds smaller than 
>> print dialog.
>> Therefore, I removed this check from ServiceUI.
>>
>> Please find the updated webrev:
>> http://cr.openjdk.java.net/~psadhukhan/8138749/webrev.03/
>>
>> Regards
>> Prasanta
>>> -phil.
>>>
>>>
>>>> Please find the updated webrev
>>>> http://cr.openjdk.java.net/~psadhukhan/8138749/webrev.02/
>>>>
>>>> Regards
>>>> Prasanta
>>>> On 1/22/2016 4:59 AM, Phil Race wrote:
>>>>> That is definitely better .. although I wonder if it would be even 
>>>>> better
>>>>> if you found when there is an 'active' window that you tried to
>>>>> position the dialog such that it is near the upper-left corner of 
>>>>> that window ?
>>>>> Please experiment and see how that looks compared to what other 
>>>>> apps are doing.
>>>>>
>>>>> -phil.
>>>>>
>>>>> On 01/21/2016 01:45 AM, prasanta sadhukhan wrote:
>>>>>> Hi Phil,
>>>>>>
>>>>>> Please find the updated webrev
>>>>>> http://cr.openjdk.java.net/~psadhukhan/8138749/webrev.01/
>>>>>> which do away with the change in ServiceUI.
>>>>>>
>>>>>> Regards
>>>>>> Prasanta
>>>>>> On 1/21/2016 2:59 AM, Phil Race wrote:
>>>>>>> The changes in ServiceUI seem like they can cause the spec. to 
>>>>>>> be contradicted.
>>>>>>> It says
>>>>>>>
>>>>>>>    * @param gc used to select screen. null means primary or 
>>>>>>> default screen.
>>>>>>>
>>>>>>> but you update the code such that it will use the active window.
>>>>>>> If that is on a secondary screen but null was passed in.
>>>>>>>
>>>>>>> I am not entirely sure why ServiceUI needs to be changed at all.
>>>>>>> It is just the caller of ServiceUI ..
>>>>>>>
>>>>>>> -phil.
>>>>>>>
>>>>>>>
>>>>>>> On 01/20/2016 03:08 AM, prasanta sadhukhan wrote:
>>>>>>>> Hi All,
>>>>>>>>
>>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8138749
>>>>>>>> webrev: http://cr.openjdk.java.net/~psadhukhan/8138749/webrev.00/
>>>>>>>>
>>>>>>>> Please review a fix for a long standing issue whereby it is 
>>>>>>>> seen that
>>>>>>>> PrinterJob.printDialog(attr set) does not support multi-monitor 
>>>>>>>> setup. When this API is invoked, the print dialog is always 
>>>>>>>> displayed on the default screen device regardless of where the 
>>>>>>>> application is running.
>>>>>>>> This is because this method
>>>>>>>> uses ServiceDialog class for creating the dialog and that 
>>>>>>>> indeed supports passing a GC in which we would like to have the 
>>>>>>>> dialog. But printer job always uses the GraphicsConfig of the 
>>>>>>>> default screen device
>>>>>>>> resulting in print dialog to be shown on primary device/monitor.
>>>>>>>>
>>>>>>>> I have not considered pageDialog() for this fix. Will create a 
>>>>>>>> separate bugid and send a patch for that as well once this fix 
>>>>>>>> is approved.
>>>>>>>>
>>>>>>>> Regards
>>>>>>>> Prasanta
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>
>