[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
Mon Feb 15 11:39:05 UTC 2016
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