[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
Wed Mar 2 16:03:52 UTC 2016
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