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

Philip Race philip.race at oracle.com
Thu Feb 18 00:23:09 UTC 2016

I don't think we should be adjusting the width of the dialog so that 
part of the change
should be removed which will simplify the logic.

Of course anyone calling the ServiceUI public API does not have any
insight into the size of the dialog since it is not explicitly returned 
so they
could inadvertently position it so that it is partly off the bottom or 
right side of the screen.
Perhaps in a multi-monitor environment with two side by side screens 
they want
the dialog centred such that it spans the two
Perhaps they want these behaviours but I suspect not so it is probably worth
'fixing up' some of these cases.

So our 'tweaking' of the position should be limited so that it positions 
the dialog
such that its moved the minimum amount to that the bottom right is on 
(the specified) screen.
If - and only if - that would cause it to be partially off the top-left 
- possible only
if the screen is smaller than the dialog, should we adjust the dialog 
position in
a way that allows the bottom or right to be off-screen.

I think we are mostly there and it is not important enough to add new API to
ServiceUI at this time to give us hints about how to handle such cases - 
I have given it some consideration.

There are lot of things we *could* do here
- expose the pageDialog (now commented out)
- provide a way to pass in the owner Dialog or Frame and stop ignoring 
it in ServiceDialog.
    That would be useful for modality.
- provide those hints I mentioned

I think #1 and #2 on that list might be the most useful to some people but
don't invalidate this fix even if it might need to be revised if we ever 
do them ..


On 2/15/16, 3:39 AM, prasanta sadhukhan wrote:
> Hi Phil,
> >> Line 836 should be
> >> x = (bounds.x + bounds.width) - dlgBound.width
> can be wrong if (bounds.x + bounds.width) - dlgBound.width comes out 
> to be negative (or less than window top-left x,y coordiantes)
> For example, if window bounds for secondary monitor is (x=1600, y=0, 
> width=489, height =361)
> and dialog bounds is (x=1650, y=50, width=460, height=353) then above 
> calc will set x to (1629, 8) [ x=1600+489-460=1629 , y = 0+361-353=8] 
> which is ok
> but if we have dialog bounds (x=1650, y=50, width=554, height = 390) 
> then x becomes 1600+489-554=1535 and y becomes 0+361-390= -29 so it 
> will go beyond the top-left corner of window.
> so for those cases, I am
> setting x and y to bounds.x, bounds.y so that atmost it can go to 
> top-left and not beyond that and
> also setting the dialog width /height to window width/height (as if we 
> keep the same dialog width,height it will become 1600,0,554,390 which 
> is again going beyond window bounds 1600,0,489,361)
> Please find the updated webrev
> http://cr.openjdk.java.net/~psadhukhan/8138749/webrev.05/
> Regards
> Prasanta
> On 2/11/2016 4:29 AM, Phil Race wrote:
>> if (dlgBounds.x + dlgBounds.width > bounds.x + bounds.width) {
>>  836                 x = bounds.x;
>> Line 836 should be
>> x = (bounds.x + bounds.width) - dlgBound.width
>> Same logic for y.
>> 841             pageDialog.setBounds(x, y, bounds.width, bounds.height);
>> That looks wrong. It is setting the width/height of the dialog to
>> be the width/height of the screen.  Should be dlgBounds.width + height.
>> -phil.
>> On 02/05/2016 12:57 AM, prasanta sadhukhan wrote:
>>> On 2/4/2016 11:01 AM, prasanta sadhukhan wrote:
>>>> Hi Phil,
>>>> On 2/3/2016 11:49 PM, Phil Race wrote:
>>>>> Hi,
>>>>> I think it is worth addressing all this at once and I think it can 
>>>>> be done
>>>>> entirely in RPJ, leaving ServiceUI alone. Ie fix it just for the 
>>>>> case where
>>>>> RPJ displays the dialog(s). 
>>>> RPJ display the dialog only for pageDialog
>>>> http://hg.openjdk.java.net/jdk9/client/jdk/file/b9a92b2a8b37/src/java.desktop/share/classes/sun/print/RasterPrinterJob.java#l820 
>>>> whereas ServiceUI display the printDialog
>>>> http://hg.openjdk.java.net/jdk9/client/jdk/file/b9a92b2a8b37/src/java.desktop/share/classes/javax/print/ServiceUI.java#l227 
>>>> So, are you saying we fix it only for pageDialog and leave out 
>>>> printDialog()?
>>> Please find updated webrev
>>> http://cr.openjdk.java.net/~psadhukhan/8138749/webrev.04/
>>> where both RPJ and ServiceUI is modified to show the dialog entirely 
>>> on-screen.
>>> I could not leave ServiceUI without modification as
>>> // 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);
>>>         }
>>> this code was drawing the dialog in primary so I have to delete the 
>>> code to make sure dialog is displayed on secondary to fix the bug. 
>>> And since anyway, I was modifying ServiceUI, I thought of adding the
>>> check to draw the printDialog on screen in ServiceUI same as 
>>> pageDialog.
>>> Regards
>>> Prasanta
>>>> Regards
>>>> Prasanta
>>>>> When an application calls ServiceUI directly, it
>>>>> can decide where it would like the dialog subject to the existing 
>>>>> constraint.
>>>>> The test really should be one test so that the behaviours can be
>>>>> tested as one with lower overhead.
>>>>> -phil.
>>>>> On 02/02/2016 09:34 PM, prasanta sadhukhan wrote:
>>>>>> 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

More information about the 2d-dev mailing list