Recent

Author Topic: FPVectorial merge request  (Read 1196 times)

Hansi

  • New Member
  • *
  • Posts: 12
FPVectorial merge request
« on: December 30, 2022, 07:12:55 pm »
Hi!

I've added a little update to FPVectorial and created the GitLab merge request

https://gitlab.com/freepascal.org/lazarus/lazarus/-/merge_requests/131

Unfortunately I don't know if everything is alright, or if I still have to do something. The CI/CD pipeline didn't succeed (even after I added my credit card number  :o ). Will the merge request still be seen by a Lazarus maintainer and merged?

Please let me know what to do next or any other comment.

Thanks
  Hansi

Bart

  • Hero Member
  • *****
  • Posts: 4971
    • Bart en Mariska's Webstek
Re: FPVectorial merge request
« Reply #1 on: December 30, 2022, 07:34:10 pm »
The pipeline failure seems to have nothig to do with your code changes.

Bart

wp

  • Hero Member
  • *****
  • Posts: 10851
Re: FPVectorial merge request
« Reply #2 on: December 30, 2022, 08:04:31 pm »
I'm not a git specialist, but I suspect that, since you are not a team member, you do not have the permission to merge your changes to the main branch directly. Write a bug report, and a team member will review your changes and merge them. Your credit card is not needed to participate in the development. ;D

Briefly looked over your changes. Having the font size as a floating point number suddenly, I guess this will this break existing code. Moreover there is the issue that the float-to-string conversions can create strings without point as decimal separator. The odt writer, for example, has a special function FloatToODTText for correct conversion to text, or if you prefer usage of the Format function, it contains a dedicated formatsettings record, named FPointSeparator. I did not look at the other writers, I guess it's similar there.

Hansi

  • New Member
  • *
  • Posts: 12
Re: FPVectorial merge request
« Reply #3 on: December 30, 2022, 08:55:01 pm »
Hi!

Thanks Bart and wp for your feedback!

I'm not a git specialist, but I suspect that, since you are not a team member, you do not have the permission to merge your changes to the main branch directly. Write a bug report, and a team member will review your changes and merge them. Your credit card is not needed to participate in the development. ;D

Yes, but is the existence of a merge request alone sufficient, or do I have to create a GitLab issue? Or even poke a maintainer directly?

Briefly looked over your changes. Having the font size as a floating point number suddenly, I guess this will this break existing code. Moreover there is the issue that the float-to-string conversions can create strings without point as decimal separator. The odt writer, for example, has a special function FloatToODTText for correct conversion to text, or if you prefer usage of the Format function, it contains a dedicated formatsettings record, named FPointSeparator. I did not look at the other writers, I guess it's similar there.

Regarding existing code: Assigning an integer to a double is ok because of auto conversion (e.g., when calling AddText or setting a member variable). IMHO, the only interesting case is when a programmer reads back the font size from TvText.Font.Size. I guess this should be mentioned in the release notes.

Regarding decimal separator: Thanks for the hint, I have missed that detail! Fortunately, even on my German setup everything worked fine. But I'll rework the changes and update the merge request accordingly.

On a side note: It seems that the repository doesn't contain all files, because I couldn't find a single user of GenerateDebugTree, despite it being a very elaborate infrastructure behind. Perhaps also some self-tests exist (more than the examples).

Thanks
  Hansi

Bogen85

  • Hero Member
  • *****
  • Posts: 572
Re: FPVectorial merge request
« Reply #4 on: December 30, 2022, 09:35:33 pm »
Yes, but is the existence of a merge request alone sufficient, or do I have to create a GitLab issue? Or even poke a maintainer directly?

You should always create a GitLab issue for things like this.
The PR would be linked to in the initial issue report as a proposal for how you feel the issue could be resolved.

Typically the Lazarus forum or contacting developers directly is not the preferred (as it is usually impractical) method for change related item discussions like this.
The GitLab issue tracker is.

But of course not everyone can automatically be expected to be familiar with the process, and asking here is good!

« Last Edit: December 30, 2022, 09:44:01 pm by Bogen85 »

Hansi

  • New Member
  • *
  • Posts: 12
Re: FPVectorial merge request
« Reply #5 on: December 30, 2022, 10:15:36 pm »
Hi!

Yes, but is the existence of a merge request alone sufficient, or do I have to create a GitLab issue? Or even poke a maintainer directly?

You should always create a GitLab issue for things like this.
The PR would be linked to in the initial issue report as a proposal for how you feel the issue could be resolved.

Thanks, done: https://gitlab.com/freepascal.org/lazarus/lazarus/-/issues/40073

Bye
  Hansi

wp

  • Hero Member
  • *****
  • Posts: 10851
Re: FPVectorial merge request
« Reply #6 on: December 31, 2022, 01:15:30 am »
Applied, thank you.

How familiar are you with FPVectorial? This package is badly broken, and more contributions to fix the issues are very welcome.

I couldn't find a single user of GenerateDebugTree, despite it being a very elaborate infrastructure behind.
The debug tree is displayed in an external application on ccr, fpvviewer (a lot of years ago, fpvectorial was available on ccr, and this is a remnant of it). You see the debug tree after loading an svg file and clicking on "View FPVectorial tokens". You can find the application at https://sourceforge.net/p/lazarus-ccr/svn/HEAD/tree/applications/fpvviewer/

Hansi

  • New Member
  • *
  • Posts: 12
Re: FPVectorial merge request
« Reply #7 on: December 31, 2022, 03:16:48 pm »
Hi!

Applied, thank you.

Thank you too!

How familiar are you with FPVectorial? This package is badly broken, and more contributions to fix the issues are very welcome.

Using it for a few weeks now (with long breaks in between) to draw diagrams of measurements, see https://github.com/hansiglaser/pas-gpib/blob/master/examples/comparison/vectorialdiagram.pas, diagrams.pas, and to display it in a GUI (mainform.pas) and save SVGs to be included in a LaTeX report (report.pas).

I didn't have any real issues so far. Scaling and text (size&position) to be "beautiful" in the GUI and in SVGs doesn't work properly yet, but this is my own fault, because I have too many magic numbers around.
One problem is that the bounding box of the SVGs is off, often way off, but I don't understand enough. Fortunately, inkscape can use the real objects when converting to PDF.
Another drawback is that it relies on TCanvas to calculate text outlines, but I assume there is no better way. It is however a problem for my command line tool which just generates the SVGs, which adds a dependency which I still don't have provided.
Ah, and I tried TTvFormula to write "105" (with a true exponent), and this didn't work ("// this doesn't raise the exponent, it is written in the same line and same height as the base"), but perhaps I just didn't provide the Canvas early enough. I've resorted to hand crafted placement.

I couldn't find a single user of GenerateDebugTree, despite it being a very elaborate infrastructure behind.
The debug tree is displayed in an external application on ccr, fpvviewer (a lot of years ago, fpvectorial was available on ccr, and this is a remnant of it). You see the debug tree after loading an svg file and clicking on "View FPVectorial tokens". You can find the application at https://sourceforge.net/p/lazarus-ccr/svn/HEAD/tree/applications/fpvviewer/

Thanks for the info! Perhaps would be worth mentioning in a README file or (more explicitly) in the great Wiki page. I've cobbled together a tiny implementation which just executes WriteLn (in fpvwritetest.pas but didn't commit). Should I extract it to a separate debugtest.pas? I'll attach the diff, if you want to review my attempt.

Thanks
  Hansi

wp

  • Hero Member
  • *****
  • Posts: 10851
Re: FPVectorial merge request
« Reply #8 on: December 31, 2022, 04:19:14 pm »
First of all: I am not the original author of fpvectorial - this is Felipe Monteiro de Carvalho, but he left the team quite some time ago. Since I had spent some time in svg and wfm support I feel responsible to continue his work, at least to some degree.

svg is quite buggy ATM in rendering. Lots of my test images of that time do not render correctly any more after Felipe had changed some settings in the coordinate system, but I cannot remember the details.

For drawing diagrams of measurement data I would recommend to use TAChart which you can instruct to render to an svg file (project "savedemo" in folder components/tachart/demo/save of your Lazarus installation). See also https://wiki.lazarus.freepascal.org/TAChart and the links on this page; there's a lot of tutorials to explain how to use it.

[EDIT]
I added a sample project, based on your .diff file above, to demonstrate the debug tree.
« Last Edit: December 31, 2022, 05:01:14 pm by wp »

Hansi

  • New Member
  • *
  • Posts: 12
Re: FPVectorial merge request
« Reply #9 on: December 31, 2022, 10:16:54 pm »
Hi!

First of all: I am not the original author of fpvectorial - this is Felipe Monteiro de Carvalho, but he left the team quite some time ago. Since I had spent some time in svg and wfm support I feel responsible to continue his work, at least to some degree.

Thanks for the background information. I know Felipe very well from watching the FPC mailing lists, but now that you say, his last message is from 2017-07-31.

svg is quite buggy ATM in rendering. Lots of my test images of that time do not render correctly any more after Felipe had changed some settings in the coordinate system, but I cannot remember the details.

Oh really? My only problems are with text (but this is due to the former rounding and missing TCanvas) and the overall bounding box. But perhaps I use it in a very simple way.

One other thing I noticed when looking at the SVG source code as well as the debug tree is that TvText always has a newline appended. This seems to come from the use of TStringList.Text, which concatenates all strings and appends a newline, even if it only has one single short piece of text. But it didn't cause any harm so far.

For drawing diagrams of measurement data I would recommend to use TAChart which you can instruct to render to an svg file (project "savedemo" in folder components/tachart/demo/save of your Lazarus installation). See also https://wiki.lazarus.freepascal.org/TAChart and the links on this page; there's a lot of tutorials to explain how to use it.

I had looked into that, but I needed some very special things like horizontal boxes, semilogarithmic scale with negative (!) values, ... So I figured it is simpler to wrap FPVectorial and directly draw the diagram myself.

[EDIT]
I added a sample project, based on your .diff file above, to demonstrate the debug tree.

Very nice, thanks!

Bye
  Hansi

Hansi

  • New Member
  • *
  • Posts: 12
Re: FPVectorial merge request
« Reply #10 on: January 05, 2023, 11:21:20 pm »
Hi again!

I've now added support for pen and brush alpha/opacity, see my commit
  https://gitlab.com/hansiglaser/lazarus/-/commit/1268a1bb8a0f7afb28f50709b372a21bb7e00762

Unfortunately, when I create a merge request, then it wants to include all 3 commits, and not only that one new. I also found no way to update my fork with the latest and greatest original (except "mirroring", which is a premium feature and requires that I didn't commit into the main branch).

Please let me know how I can help you to merge this little change.

Thanks
  Hansi

wp

  • Hero Member
  • *****
  • Posts: 10851
Re: FPVectorial merge request
« Reply #11 on: January 06, 2023, 12:18:59 am »
Committed and pushed. Thanks.

Hansi

  • New Member
  • *
  • Posts: 12
Re: FPVectorial merge request
« Reply #12 on: January 06, 2023, 03:38:18 pm »
Hi!

Thank you too!

Bye
  Hansi

 

TinyPortal © 2005-2018