"Build from source": all report properties not properly set

Indigo744 commented 3 years ago

Steps to reproduce:

Create an empty ACCDB file
Add a report, change printer options (landscape, paper size...)
Export to VCS
Build from source
Open report, printer options were not set back:

By doing a diff between both reports\État1.json output, I can see that for the first one, the printer properties are propely set:

{
  "Items": {
    "Printer": {
      "Orientation": "Landscape",
      "PaperSize": "A5",
      "DefaultSource": 257,
      "PrintQuality": 600,
      "Color": "Monochrome",
      "Duplex": "Horizontal",
      "Resolution": 600,
      "MediaType": 257
    }
}

But for the second one, everything is reset:

{
  "Items": {
    "Printer": {
      "Orientation": "Portrait",
      "PaperSize": "A4",
      "PaperLength": 2970,
      "PaperWidth": 2100,
      "DefaultSource": "Auto",
      "PrintQuality": 600,
      "Color": "Monochrome",
      "Duplex": "Horizontal",
      "Resolution": 600,
      "MediaType": 257
    }
}

Thank you.

joyfullservice commented 3 years ago

Yes, this is an outstanding issue. I worked through the code to extract and save the printer settings, but have not yet finished out the part of applying them back to reports after building from source.

Indigo744 commented 3 years ago

Ah ok. If I find some time, I'll try to have a go at it with a PR...

Indigo744 commented 3 years ago

So I tried to have a look at it, but I'm a little out-of-depth here...

There is an clsDbReport.ImportPrintVars function with some commented-out code.

This code uses a clsDevMode.DictionaryToDevMode function, but is seems to be a private one.

So I wonder if I should instead implements something in the clsDevMode.LoadFromJsonFile function instead.

In any case, I'm not sure how I can go from this clsDevMode to the proper properties in the report.

Could you point me in the right direction?

joyfullservice commented 3 years ago

@Indigo744 - Thanks for your time in reviewing this! As you may have seen, the whole print vars/saved report settings is quite a complicated project to tackle. I created the clsDevMode class several months ago to handle some of the more complex aspects of reading and writing to the DevMode structures, but have not yet finished the side of writing the values back to the report.

There are a few different approaches that could be taken here. The direction I was heading with it was to start by setting the correct printer for the report, then using the Access API to apply the saved settings that I can through the available properties, and then to use the DevMode structure to set any final items that are not exposed through the object model.

In the long term I am also needing this for some projects that I am working on, so I went ahead and started working on this again yesterday, and I am making some progress. Hopefully it all works just as it should, but printer drivers can be fun... 😄

Indigo744 commented 3 years ago

Alright I'll let you handle this, since you'll be able to do things way better than I would 😉

joyfullservice commented 3 years ago

Sounds good! I have made some good progress this morning.

Indigo744 commented 3 years ago

Just tell me if you need some testing on my side, I'll be glad to help.

joyfullservice commented 3 years ago

I just rolled out an initial version of the code to write saved print settings back to a report object. I haven't done much testing with it, but we should be getting close. 😄

joyfullservice commented 3 years ago

Successfully tested the restoring of a user-defined form name, which involved updating the raw .PrtDevMode structure. Let me know how the testing goes on your end!

Indigo744 commented 3 years ago

Thank you!

Here are the result:

Selecting a specific printer with some parameters (MS Print To PDF): OK ✅ Before/After:
Selecting the default printer, but different parameters: NOK ❗
Before: After:

Indigo744 commented 3 years ago

Would you like a test ACCDB?

joyfullservice commented 3 years ago

Thank you for the screenshots and further clarification! I had been focusing on restoring print settings when a specific printer was assigned to the report, but did not deal with the case where you are printing to the default printer, but still want to preserve some of the print options such as paper source. That shouldn't be too hard to implement.

Indigo744 commented 3 years ago

Ah ok, that explains why I still see the issue. Thing is, most of my reports are using the default printer but with different paper size or orientation 😸 so I'm more interested in the latter 😉

But it is looking good! Can't wait to have all fixes in so I can better track the app in TFS! I'm so happy to have found this project 😄

joyfullservice commented 3 years ago

The more I think about it, the more I am coming to the conclusion that the print settings should always be written back to the report after importing. Since we are stripping them out in the sanitized source file, and recording them in a more human-readable JSON format, they need to be restored to the report from the JSON file after importing. Some common settings, such as paper size, are stored in the Printer object of the report, and must be restored from the JSON file to properly reconstruct the report. This is the case even when the report does not have a specific printer assigned.

@Indigo744 - I have added commit 9caf0ae to always apply the printer settings when building from source. Could you test that with your database and reports and see if this has the desired effect? If the build performance is seriously impacted, there are some potential things we could do to help with that, but I would be curious to know how it works for you.

Indigo744 commented 3 years ago

After some test, it seems to be working better!

The performance is indeed impacted, going from 70 secs to 120 secs (+70%).

Regarding the field, there are still some weird fields that doesn't seem to be properly set. For instance:

The thing is, I'm not really sure how these fields really impact the report printing. Reports looks the same. I can't find a way to set them through the UI, so maybe it's not relevant.

Indigo744 commented 3 years ago

Another field that are not set back: PaperLenght/Width:

But the paper size set in the UI is still the same (it come from PaperSize property I guess) so it's should be OK:

joyfullservice commented 3 years ago

At this point I think we have achieved the goal of applying print settings from the exported JSON file back to report objects created when building from source. From here, I think there are a few related new issues/enhancements that could be created as separate issues.

1. Build performance. Opening a report in design view and setting the properties does work, but it is obviously slow, due to the loading of the reports into the designer. There are a couple ways this could potentially be addressed. We could reconstruct the binary DevMode structure and write that section back to the import file just before importing. This would make the import very fast, but it could be a bit complex to do. Another approach would be to load the current default printer settings into the class, and only modify the reports if they have important settings that differ from the default settings.

2. Filter out unneeded print settings. How many people are actually saving image color matching properties for their print driver to a specific Access report? Or how about TTF font handling? Very few, if any people probably care about those finer details, so these could be potentially filtered out in the options. I am picturing a list of printer settings that you can optionally save with your reports. For most, they probably just need the basics like orientation, paper size, and maybe paper source and duplexing. For those that want the finer details, they could check the additional boxes. In my code, I went ahead and included all of the settings for completeness, but this may cause some unintended noise in the diff files when exported from systems with different printers.

3. Save print settings for forms. The same printer settings that you use with reports can also be used with forms. Since this is probably a very uncommon need, I am fine with leaving this as a feature request that someone can make when the need arises. (Or if someone just has lots of time and is looking for a project. 😄)

I am proposing that we close this issue as resolved, and create new issues for any of the three items above as needed. I am happy to review pull requests if anyone wants to work on these issues, but I don't have a pressing need for them myself.

Indigo744 commented 3 years ago

Thank you for your hard work on this. It is clearly a complicated thing.

Juste a couple of question before moving on if you don't mind!

Build performance: there is a thing I actually don't understand: why can't we simply use the DevMode binary structure that is exported by the SaveAsText function? I can see there is a PrtDevMode (and PrtDevModeW). If I simply perform a SaveAsText and LoadFromText, I rightfully get the correct printer values set. Maybe we could keep this value around (in JSON? in bas?) if we need it. Or even behind a configuration flag?
Filter out unneeded print settings. This would be good and hopefully simple to implement. I can even see a simple textbox where we define a list of option name separated by commas that we want to be always set back. I, for one, would only need a few (mainly orientation, maybe paper size).
Save print settings for forms. I didn't even know it was possible, and I don't even want to know more. I completely agree with you 😅

joyfullservice commented 3 years ago

@Indigo744 - Great question! Long before I got involved in the project, the code was set to strip out the PrtDevMode sections when saving the source code. From what I have seen, I think this was primarily because this information would change frequently and caused unwanted "noise" in version control. See here and here for a couple other discussions on this topic. I have not personally tested to see if this happens at every export, or just when the report is printed, or just when the report design is modified.

The other challenge is that the actual print information is stored in a binary blob which is difficult to read and understand when reviewing a diff file. (I shouldn't need to use a hex editor to determine if my report was switched from portrait to landscape orientation.) I like having the JSON representation for that reason. The print margins have the same challenge. Without a calculator and specification on the MIP structure, it would be hard to understand what changed in the screenshot above.

From a version control standpoint, probably the ideal solution is to write it back to the file before import, overlaying the JSON values onto a DevMode structure pulled from the appropriate printer. (Default printer or named assigned printer.) I believe this would have all the benefits of performance on the build, and readability in version control. And I really don't think it is that far out of reach. Essentially we just need to convert the DevMode structure back into a binary blob and write it back to the file with the appropriate section headings and indentation. Whether we need to create both the ANSI and Unicode versions would be a matter of testing. (I would imagine this would be to support Unicode printer names...)

Even after working out the code to write the print settings back to the export file, I suppose it probably still would be helpful to also have the option to filter out unneeded print settings. Hmm... Perhaps it would be worth taking a little more time to implement this approach of reconstructing the PrtDevMode blob... 🤔

Indigo744 commented 3 years ago

Ah, I understand. It makes sense! Tracking binary data is not really nice from a VCS point of view.

But what about having both? The JSON parsed value for easier diffing, and the binary part for easier import? It could be behind a flag "keep printer binary blob for fast import" (with pros and cons in the documentation).

I feel like trying to recreate the binary blob from scratch will be an herculean tack with a lot of testing needed, which could turn into a lot of bug reports...

In any case, I'll try to help as much as I can 😸

gilbertbw commented 3 years ago

@Indigo744 - Great question! Long before I got involved in the project, the code was set to strip out the PrtDevMode sections when saving the source code. From what I have seen, I think this was primarily because this information would change frequently and caused unwanted "noise" in version control. See here and here for a couple other discussions on this topic. I have not personally tested to see if this happens at every export, or just when the report is printed, or just when the report design is modified.

I work on https://github.com/x-ware-ltd/access-scc-addin you link to. The noise in commits is exactly why we removed the blocks. In testing we realised that removing Prt sections for forms/reports was a bad idea (as page size and orientation should be committed into source code control!) and commented out that code.

The other challenge is that the actual print information is stored in a binary blob which is difficult to read and understand when reviewing a diff file. (I shouldn't need to use a hex editor to determine if my report was switched from portrait to landscape orientation.) I like having the JSON representation for that reason. The print margins have the same challenge. Without a calculator and specification on the MIP structure, it would be hard to understand what changed in the screenshot above.

From a version control standpoint, probably the ideal solution is to write it back to the file before import, overlaying the JSON values onto a DevMode structure pulled from the appropriate printer. (Default printer or named assigned printer.) I believe this would have all the benefits of performance on the build, and readability in version control. And I really don't think it is that far out of reach. Essentially we just need to convert the DevMode structure back into a binary blob and write it back to the file with the appropriate section headings and indentation. Whether we need to create both the ANSI and Unicode versions would be a matter of testing. (I would imagine this would be to support Unicode printer names...)

After our failed attempts to remove the printer blocks, I have done some work (not yet merged in) on serialising and deserialising the printer blocks instead that may be of interest. modPrinter handles serialising and deserialising PrtDevMode and PrtMip. I chose to store these inline in the form under sections called SerializedPrtDevMode and SerializedPrtMip respectively.

Even after working out the code to write the print settings back to the export file, I suppose it probably still would be helpful to also have the option to filter out unneeded print settings. Hmm... Perhaps it would be worth taking a little more time to implement this approach of reconstructing the PrtDevMode blob... 🤔

My plan once (de)serialising the printer blocks is completed is to add exactly this to our project, to exclude device specific settings and leave only page layout type information that you would usually keep under SCC.

joyfullservice commented 3 years ago

@gilbertbw - Thanks for chiming in! Thank you also for the work you have done on the x-ware-ltd/access-scc-addin project. This gave me some helpful tips and inspiration on dealing with the print settings when I was working on that back in May.

Thank you also for the link on your recent work with serializing/deserializing the binary sections. The CopyMemory approach on the byte conversion looks a whole lot simpler and cleaner than what I was picturing in VBA... 😄

You probably noticed how I took the approach of using the JSON format for settings not contained in the export files themselves. This was mainly for the following reasons:

JSON is a text-based format that is very familiar with developers, highly readable, and very easy to interact with using Dictionary objects. I felt that it made a great universal file format for storing structured data in version control.
There are a number of things in Access that don't have a consistent and direct way to export them. Some examples are VBE Project options, linked table connections, IMEX import/export specifications, relationships, database properties, database documents (and embedded properties), navigation pane groups, VBE project references, etc... JSON gave me a consistent file format for storing all this structured data with the project in version control.
It also gave me a consistent format to use when rebuilding the database project from source files. (See supported objects)

Also, I was just curious, but have you done any testing with the joyfullservice/msaccess-vcs-integration addin? I would be very interested in your feedback on the performance and completeness of the export. You can also test the Build feature which allows you to reconstruct the database application from source files. It is very simple to install. Just download and run the .accda file from Releases, then open a database and click Version Control from the add-in menu.

joyfullservice commented 3 years ago

@Indigo744 - Great news! I was able to build a function to successfully recreate the binary print setting blob sections from data loaded into the clsDevMode class. This means that now we can take the approach I described above to dynamically restore the print settings into the export files just before importing them. This eliminates the need to open the reports in design view, which is where we were seeing the performance hit.

Now that we have the building blocks in place, here is how I envision this working in the build process.

Check the JSON print settings to see if a specific printer was specified.
If so, load clsDevMode from the specified printer, otherwise load from the default printer.
Apply the JSON print settings to clsDevMode. (Needs new function added to do this, similar to SetPrinterOptions, except that we are modifying m_tDevMode, not the report object.)
Call the GetPrtDevModeBlock() and other related functions to get the formatted export sections.
Create a temp file, combining the export file with the newly generated print sections into the new file.
Import the new file into the database.
Delete the temp file.

At this point you should now have all of your saved print settings in the imported report. 😄 If you would like to put the pieces together and do some testing, I would be happy to collaborate on a pull request!

Here is some example code:

Public Sub TestBlob()

    With New clsDevMode
        .LoadFromDefaultPrinter
        '.LoadFromReport Reports(0)
        '.LoadFromExportFile "C:\Users\zzz\Test.bas"
        '.LoadFromExportFile "C:\Users\zzz\Test2.bas"
        Debug.Print .GetPrtDevModeBlock
        Debug.Print .GetPrtMipBlock
        Debug.Print .GetPrtDevNamesBlock
        Debug.Print ConvertToJson(.GetDictionary, "  ")
    End With

End Sub

Indigo744 commented 3 years ago

@joyfullservice this is really neat! I'll try to have a look into it this afternoon and eventually open a PR to start working on it 😸

Thank you very much!

Indigo744 commented 3 years ago

@joyfullservice I'm very sorry I wasn't able to start working on it as promised. France was going in another lockdown and at work it was a bit... frantic to prepare everything for full remote work.

I can see you made some progress. I'll pull it and start looking into it.

joyfullservice commented 3 years ago

No worries! I was glad I took a little more time on it since it turned out to be a little more involved that I was originally anticipating. Maybe I will see if I can get the concept roughed out, and you can do some testing and refining on your side.

Sorry to hear of the challenges with another lockdown. This really has been an interesting year! I am grateful that in our line of work we at least have the option of being able to work remotely. So many others just lost their jobs or had their entire line of work essentially shut down during the pandemic.

joyfullservice commented 3 years ago

Okay, I have just completed the initial implementation of the process described above. It will need further testing and possible refinement before we roll it out to the masses, but please feel free to submit pull requests for any changes that are needed. 😄

joyfullservice commented 3 years ago

Also, as a side note, I also adjusted the sanitize function in 5129240 to remove the binary PrtMip (print margins) section. We have the needed detail fully saved in the JSON file, so there is no need to leave this block in the source file, since it can cause undesired "noise" in version control when reserved values change.

Indigo744 commented 3 years ago

Hello,

I am testing the latest version, and I must say I'm impressed with all the improvements! It's faster, better... Thank you so much!

Regarding the printer part, I now notice a strange behavior. If I export my original file, I get in the JSON:

    "Printer": {
      "Orientation": "Portrait",
      "PaperSize": "A4",
      "DefaultSource": 278,
      "PrintQuality": "Medium",
      "Color": "Color",
      "Duplex": "Simplex",
      "TTOption": "Bitmap",
      "FormName": "A4",
      "ICMMethod": "None",
      "ICMIntent": "Contrast",
      "MediaType": "Standard"
    },

Now if I build from source, then export again, I get:

    "Printer": {
      "DeviceName": "\\\\SERVER\\Shared Printer Name",
      "Orientation": "Portrait",
      "PaperSize": "A4",
      "PaperLength": 11.69,
      "PaperWidth": 8.27,
      "DefaultSource": 278,
      "PrintQuality": "Medium",
      "Color": "Color",
      "Duplex": "Simplex",
      "Resolution": 600,
      "TTOption": "Substitute Device Font",
      "FormName": "A4",
      "ICMMethod": 0,
      "ICMIntent": 0,
      "MediaType": "Standard"
    },

Now the ICMMethod, TTOption,... params don't bother me as I don't think they play any significant role. But I'm more concerned for the DeviceName param which appeared: it is actually the name of my default printer, which is a shared network printer at work.

The thing is, in the report settings, it still shows "Default printer":

if I change my default printer to "MS Print to PDF" and restart my app, the settings stay the same...

So I don't know. It just feels weird to have this information in the VCS.

joyfullservice commented 3 years ago

Okay, I decided to go ahead and bite the bullet and set up the user options for print settings...

Let me know how this works on your end... 😄

Indigo744 commented 3 years ago

It works really well! Thank you!

I think this issue can now be close. Thank you again for taking the time to do this... 😸

joyfullservice / msaccess-vcs-addin

"Build from source": all report properties not properly set #70