On Fri, 8 Jul 2022 15:07:46 GMT, Kevin Rushforth <k...@openjdk.org> wrote:
>> This fixes the issue that the default printer or set of available printers
>> is fixed from the first call to it in the lifetime of the application.
>> Now subsequent calls will check to see if there are changes.
>> A manual test is provided for verifying this - it requires you to add/remove
>> printers at the system level to verify anything related to this fix.
>
> modules/javafx.graphics/src/main/java/com/sun/prism/j2d/PrismPrintPipeline.java
> line 138:
>
>> 136: }
>> 137: }
>> 138: lastTime = System.currentTimeMillis();
>
> Is `lastTime` intended to be the last time you updated the printers or the
> last time the `getAllPrinters` method was called? If the former, then you
> would want to set `lastTime` right after `updatePrinters`. If the latter,
> then it's fine where it is.
The last time we updated.
If the number of printers has changed I will always rebuild the printer list
(set) but if they are the same
what are the odds a printer was removed and another one added ?
I figured if 2 minutes has passed since we last checked it is worth it, but I
am not re-setting this every
time we called getAllPrinters() else if you kept calling it every 30 seconds ..
then we'd never get past that check.
> modules/javafx.graphics/src/main/java/com/sun/prism/j2d/PrismPrintPipeline.java
> line 192:
>
>> 190: if (oldDefaultService != null && newDefaultService != null) {
>> 191: if (!oldDefaultService.equals(newDefaultService)) {
>> 192: defaultPrinter = findDefaultPrinter(printerSet,
>> newDefaultService);
>
> Don't you also want to find the default printer in the case where the old
> `default` printer was null and the `newDefaultService` is not null?
Updating this.
> modules/javafx.graphics/src/main/java/javafx/print/Printer.java line 95:
>
>> 93: } else {
>> 94: defaultPrinter.setValue(p);
>> 95: }
>
> Minor: indentation is off for these two lines
That's what you get for trying to use an IDE ... will fix ..
-------------
PR: https://git.openjdk.org/jfx/pull/817
