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

Jayathirth D V jayathirth.d.v at oracle.com
Thu Mar 3 06:33:26 UTC 2016


Hi Prasanta,

Fix looks fine to me.  We are taking care of both having the dialog within proper monitor and also making sure that it stays within the window bounds and also consistent with native print dialog.

Thanks,
Jay

-----Original Message-----
From: Philip Race 
Sent: Wednesday, March 02, 2016 9:34 PM
To: prasanta sadhukhan
Cc: 2d-dev at openjdk.java.net
Subject: Re: [OpenJDK 2D-Dev] [9] RFR: JDK-8138749, , Revisited: PrinterJob.printDialog() does not support multi-mon, always displayed on primary

approved.

On 3/1/16, 1:21 AM, prasanta sadhukhan wrote:
> Hi Phil,
>
> Please find my modified webrev
> http://cr.openjdk.java.net/~psadhukhan/8138749/webrev.07/
>
> which takes care of the fact (as compared to webrev.06), if dialog 
> does not exceed window bounds at both left or top, do not modify 
> dialog at both x,y coordinates. Only modify that coordinate in which 
> dialog has exceeded window bounds.
>
> Regards
> Prasanta
> On 2/18/2016 11:26 AM, prasanta sadhukhan wrote:
>> Hi Phil,
>>
>> On 2/18/2016 5:53 AM, Philip Race wrote:
>>> 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.
>>>
>> Please find the updated webrev that takes care of your review 
>> comments http://cr.openjdk.java.net/~psadhukhan/8138749/webrev.06/
>>
>> As for the below proposal, I guess we could handle it as part of 
>> future enhancements, can we?
>>
>> Regards
>> Prasanta
>>> 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 - although 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 ..
>>>
>>>
>>> -phil.
>>>
>>> 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