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
