Community
Participate
Working Groups
Build Identifier: I20091210-1301 - 3.6 M4 We need to be able to copy a org.eclipse.swt.printing.PrinterData object. So PrinterData class should either offer clone() or PrinterData(PrinterData). Another option would be to make PrinterData.otherData as public. We need this in org.eclipse.jface.text.TextViewer.print(StyledTextPrintOptions), so that user selected printer options can be remembered for the session(267427). Reproducible: Always
I would like to get one or the other suggested API for 3.6.1 because otherwise we have to disable printing in parallel in order to fix bug 315932.
We cannot add new API in a point release. There are a lot of PrinterData API changes coming up for 3.7. I am marking this bug for consideration for 3.7 because it makes sense to add this type of API at the same time as we make the other changes. In order to fix 315932, you only need to persist the public fields of PrinterData, one of which is 'scope', which will keep the 'SELECTION' choice of the user. Alternatively - is it not possible to simply pass the original PrinterData object for the printing in parallel? Why do you need to clone it?
>In order to fix 315932, you only need to persist the public fields of >PrinterData, one of which is 'scope', which will keep the 'SELECTION' choice of >the user. Doesn't 'otherData' contain printer specific options that would get lost? At least that's what we saw when testing the fix for bug 267427. >Alternatively - is it not possible to simply pass the original PrinterData >object for the printing in parallel? Why do you need to clone it? In order to remember the last used print dialog settings (as requested by you in bug 267427) we have to remember the last used PrinterData but we have to reset some fields like start/end, copy count or selection - most apps do that. Now, given that the latest remembered data is also used for printing in the background thread, there's always an issue. Another idea I tried was to set the data into the dialog and then override the values with e.g. PrintDialog.setScope(int) but that doesn't work for two reasons: 1) This modifies the printer data that I've set before instead of using an internal copy. 2) PrintDialog.setCopyCount(int) is missing. >We cannot add new API in a point release. The PMC can approve exceptions.
> Doesn't 'otherData' contain printer specific options that would get lost? Ah, you are correct. I would be fixing one bug and breaking another. :( > given that the latest remembered data is also used for printing in the > background thread Can you use the original PrinterData for the printing scenario given in bug 315932 comment 0, and then save it and clear those fields? Or just clear the fields right before opening the dialog again? Or something along those lines?
(In reply to comment #4) > Can you use the original PrinterData for the printing scenario given in bug # > 315932 comment 0, and then save it and clear those fields? Or just clear the > fields right before opening the dialog again? Or something along those lines? That is the problem, how to 'save' it? :) We are forced to use the 'same' PrinterData object as it cannot be copied.
> That is the problem, how to 'save' it? :) > We are forced to use the 'same' PrinterData object as it cannot be copied. Exactly.
By 'save', I meant save in global variable, as in: fgPrinterData= data; But instead of clearing the fields right away, I think you should clear them before opening the dialog. (i.e. not right after saving the data). For example: final PrintDialog dialog= new PrintDialog(shell, SWT.PRIMARY_MODAL); fgPrinterData.startPage= 1; fgPrinterData.endPage= 1; fgPrinterData.scope= PrinterData.ALL_PAGES; fgPrinterData.copyCount= 1; dialog.setPrinterData(fgPrinterData); I think that will avoid threading issues and fix bug 315932 (plus if there are any other bugs open regarding copy count or start/end page user settings not being used).
>I think that will avoid threading issues No, it will not: the saved fgPrinterData is from the previous run, so if the printing thread is not fast then changing the values in fgPrinterData will affect that result because the printer data is not copied and accessed in the thread.
Having said that: a non-API fix would be that SWT ensures that the printing thread doesn't access the given printer data i.e. only works on a copy, see org.eclipse.swt.custom.StyledText.Printing.init().
The user would have to request a second print job before the first print job was initialized (in StyledText.Printing.init()). But one print job would always work - which I think is better than the current situation where printing portions of a file will never work. By the way, I notice I forgot to protect against null, so the code should be: final PrintDialog dialog= new PrintDialog(shell, SWT.PRIMARY_MODAL); if (fgPrinterData != null) { fgPrinterData.startPage= 1; fgPrinterData.endPage= 1; fgPrinterData.scope= PrinterData.ALL_PAGES; fgPrinterData.copyCount= 1; } dialog.setPrinterData(fgPrinterData); So all I am saying is that I think this code is better than what is there. (It actually feels like the right place to reset the fields, even if you were able to save a copy). I do agree that if a user prints something, and then decides immediately to print something else before the first print job has been completely initialized, then their selections for the first print job will be clobbered. I guess I don't think this is too likely. At least, not as likely as clobbering them for sure every time. <g> So I will talk to SSQ about new API, but I don't think we have ever done it in a point release before, and I think the suggested work-around will make the bug happen much less often. I do see, however, that the requested API would have prevented the problem in the first place.
Just noticed your comment 9. Let me see if I can make that work.
>The user would have to request a second print job before the first print job >was initialized (in StyledText.Printing.init()). >But one print job would always work Nope, because the init() happens in the non-UI thread. If the user prints a second time then it can affect the previous thread.
>>But one print job would always work >Nope, because the init() happens in the non-UI thread. If the user prints a >second time then it can affect the previous thread. But that's exactly what I said - one print job is ok, but two in quick succession might have problems. Anyhow, I can't really implement comment 9 reasonably because StyledText and PrinterData are in different packages. I would have to create a public-internal way to get at the otherData, and that seems pointless because we are planning to give an API way to copy the PrinterData anyhow. I will talk to SSQ.
>Anyhow, I can't really implement comment 9 reasonably because StyledText and >PrinterData are in different packages. Looks like today is the day we disagree ;-) In org.eclipse.swt.custom.StyledText.Printing.Printing(...) - which runs in the UI thread - all data is already copied out of the printer data and stored in fields of except for the scope. Doing the same for scope is all that needs to be done.
Oh, of course. I don't need to copy otherData because you're not going to clobber it <grin>. I'll get back to you in a few minutes.
Created attachment 171551 [details] patch Easy fix. Tested 4 or 5 times, with various option combinations.
Comment on attachment 171551 [details] patch That's what I had in mind ;-) Thanks!
I draw back the API request since I no longer need it with the given fix.
Thanks for the idea, Dani (and for persevering <g>). We should untangle the bugs. We have: 1) This bug, which requests new API to copy PrinterData for someday I think I will keep this bug open, because the request makes sense. We should do it for 3.7, when we do the other PrinterData changes. 2) Bug 297964, which reminds you to use any new API we create I think you probably want to keep this open, because (as we have seen <g>) there can be issues... 3) Bug 315932, which is the real user's bug that this patch actually fixes I think I should attach the patch to that bug, and make sure the target milestone is 3.6.1 and assign to SWT, and then have that bug reviewed. I'm not sure what the 3.6.1 reviewer requirements are (+1 or +2?), but I know you'll give +1, and I should really ask Felipe (StyledText owner) to review as well, so he can give another +1 if we need it. Sound good?
>Sound good? I agree completely this time :-)
Moved bug 315932 to SWT with target milestone set to '3.6.1'.
Before fixing this, read bug 315932 comment 10 and 11. Make sure that the implementation and the doc are consistent and clear for all methods in org.eclipse.swt.printing that take or return a PrinterData object. This is not currently the case.
