[OpenJDK 2D-Dev] [9] RFR: JDK-8138749, , Revisited: PrinterJob.printDialog() does not support multi-mon, always displayed on primary
Phil Race
philip.race at oracle.com
Wed Feb 10 22:59:35 UTC 2016
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