BGRASVGImageList saves different binary format per platform (patch available)

[dokkie8844]

The TBGRASVGImageList component saves its data in binary format:

procedure TBGRASVGImageList.DefineProperties(Filer: TFiler);
begin
  inherited DefineProperties(Filer);
  Filer.DefineBinaryProperty('Items', ReadData, WriteData, True); // <==== binary format defined
end;

Internally, WriteData() uses TStringList.Text and TXMLConfig to write the SVG images. Both TStringList.Text and TXMLConfig write text using EOL markers that depend on the current platform (e.g. Windows: #13#10; MacOS/Linux: #10). The resulting text string is then written to the lfm file in binary format (hex codes).

This means that saving a BGRASVGImageList component on Windows leads to a different lfm file than saving it on MacOS/Linux.

Why is this a problem?
When developing cross-platform, saving on Windows vs saving on MacOS/Linux leads to large version control diffs everytime. Version control tools like git cannot solve these issues, since they are faced with binary data (hex codes). For version control and reviewing updates in a cross-platform setting, this is very incovenient.

How to solve this?
The other BGRA SVG components do not suffer from this issue, as TBCSVGViewer, TBCSVGButton and TBGRASVGTheme save their data in text format. This way, tools like git can quietly deal with EOL differences. However, changing the BGRASVGImageList save format from binary to text would be a breaking change (existing lfm files will not load anymore).

Another possibility is to normalize the EOL markers in the data before it is written as binary data. This way, saving on any platform will always create the same binary output. Version control systems will not show very large differences anymore.

Pros of this solution:

• The reading methods do not need any change.
• Existing lfm files are still read correctly.
• Writing to an lfm on different platforms gives consistent results.

Con of this solution:

• Existing lfm files on Windows, when re-saved, will have a one-time update of their binary data, because the EOL marker is normalized to #10. This happens just one time and only for existing lfm files.

The attached patch file implements this solution. Please review and thanks in advance for considering applying it!

After the patch, the WriteData() method looks like:

procedure TBGRASVGImageList.WriteData(Stream: TStream);
var
  FXMLConf: TXMLConfig;
  FTempStream: TStringStream;
  FNormalizedData: string;
begin
  FXMLConf := TXMLConfig.Create(Self);
  FTempStream := TStringStream.Create;
  try
    Save(FXMLConf);
    // Save to temporary string stream.
    // EOL marker will depend on OS (#13#10 or #10),
    // because TXMLConfig automatically changes EOL to platform default.
    FXMLConf.SaveToStream(FTempStream);
    // Normalize EOL marker, as data will be saved as binary data.
    // Saving without normalization would lead to different binary
    // data when saving on different platforms.
    FNormalizedData := AdjustLineBreaks(FTempStream.DataString, tlbsLF);
    // Write normalized data, so the binary data in the lfm file
    // is consistent for different platforms.
    if FNormalizedData <> '' then
      Stream.WriteBuffer(FNormalizedData[1], Length(FNormalizedData));
    FXMLConf.Flush;
  finally
    FXMLConf.Free;
    FTempStream.Free;
  end;
end;

And the Save() method looks like:

procedure TBGRASVGImageList.Save(const XMLConf: TXMLConfig);
var
  i: integer;
begin
  try
    XMLConf.SetValue('Count', FItems.Count);
    for i := 0 to FItems.Count - 1 do
      // Below, use AdjustLineBreaks() to normalize EOL markers.
      // Otherwise, FItems[i].Text will use EOL markers depending on the
      // current platform. This leads to different binary data in the lfm file.
      XMLConf.SetValue('Item' + i.ToString + '/SVG',
        AdjustLineBreaks(FItems[i].Text, tlbsLF));
  finally
  end;
end; 

The patch:

146a147,148
>   FTempStream: TStringStream;
>   FNormalizedData: string;
148a151
>   FTempStream := TStringStream.Create;
151c154,165
<     FXMLConf.SaveToStream(Stream);
---
>     // Save to temporary string stream.
>     // EOL marker will depend on OS (#13#10 or #10),
>     // because TXMLConfig automatically changes EOL to platform default.
>     FXMLConf.SaveToStream(FTempStream);
>     // Normalize EOL marker, as data will be saved as binary data.
>     // Saving without normalization would lead to different binary
>     // data when saving on different platforms.
>     FNormalizedData := AdjustLineBreaks(FTempStream.DataString, tlbsLF);
>     // Write normalized data, so the binary data in the lfm file
>     // is consistent for different platforms.
>     if FNormalizedData <> '' then
>       Stream.WriteBuffer(FNormalizedData[1], Length(FNormalizedData));
154a169
>     FTempStream.Free;
181c196,200
<       XMLConf.SetValue('Item' + i.ToString + '/SVG', FItems[i].Text);
---
>       // Below, use AdjustLineBreaks() to normalize EOL markers.
>       // Otherwise, FItems[i].Text will use EOL markers depending on the
>       // current platform. This leads to different binary data in the lfm file.
>       XMLConf.SetValue('Item' + i.ToString + '/SVG',
>         AdjustLineBreaks(FItems[i].Text, tlbsLF));

bgrasvgimagelist.pas.patch

[circular17]

Hello @dokkie8844

Thank you for the detailed explanation and patch. Your diagnostic and proposal seem sound. Normalizing the end of lines when saving would ensure that they are stable.

Thinking about the issue, we could go further in avoiding changes in LFM files by detecting the line ending when loading the configuration and use the same line ending when saving.

@lainz Hope you're doing alright. Are you available to handle this query? I can take care of it otherwise.

Regards

[lainz]

Hi @circular17 , please take care of this.
Thanks.

[circular17]

Ok I will try to add that to my todo list

[circular17]

Hi @lainz and @dokkie8844

I've implemented the change. Though I cannot test it. Can you try and see if line ending are preserved?

Regards

[lainz]

Hi @circular17 , I've tested it this way on Windows 11:

• Download latest sources
• Rebuild lazarus
• Open a form previously saved with the old code
• Move any non visual component and save the form

It produces changes in the SVG Image List... so for me as I undestand didn't work.

[circular17]

Thanks @lainz for testing. Can you give more detail on the changes that happen?

[lainz]

Not really. Is like a hex format I can't easily read it. Also takes a lot of time to load the form I've used to test (a big form full of stuff).

Maybe you can go back the code and create a form. Then test with the new code in a git repository to track changes?

Is what I did without good results... But maybe you can understand the changes in the lfm...

[circular17]

Ok so you notice a change in the data that is generated. Can you provide the file before and after?

[lainz]

Sorry is from a commercial application. I can't share.

[circular17]

Ok. Maybe a test project if there is one already?

@dokkie8844 Can you help testing and debugging? The changes are on the dev-bgracontrols branch.

[lainz]

bgracontrols\test\test_bgrathemes\umain.lfm (original is in the repo)

this is the saved with the new code
umain.zip

[circular17]

Ah indeed there are some line endings that have been adapted, but within HTML. Ok this is in the string containing the SVG. It is an XML within the XML. I suppose it is because each SVG is stored as a TStringList. I am investigating...

[circular17]

@lainz I've added a call to adapt the line breaks inside the SVGs as well. I suppose now it will be ok. Can you give it a try?

[lainz]

@circular17, now it works perfectly =)

[circular17]

Cool! Thank you so much for testing.

Well i guess we can consider the issue done!

Thank you @dokkie8844 for the request