[OpenJDK 2D-Dev] [9] RFR: JDK-8138749, , Revisited: PrinterJob.printDialog() does not support multi-mon, always displayed on primary
prasanta sadhukhan
prasanta.sadhukhan at oracle.com
Thu Feb 18 05:56:11 UTC 2016
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