pdfalto icon indicating copy to clipboard operation
pdfalto copied to clipboard

Svg path coordinates are weird.

Open philoskim opened this issue 4 years ago • 3 comments

The following is an svg example which is produced by pdfalto 0.3.

<?xml version="1.0" encoding="UTF-8"?>
<svg xmlns="http://www.w3.org/2000/svg">
  <clipPath id="p2_s427" evenodd="true">
    <path d="M56.4,7 L539.6 L539.6 L56.4, L56.4, Z"/>
  </clipPath>
  <g style="fill: #EFF7EF;fill-rule: evenodd;fill-opacity: 1;" id="p2_s429">
    <path d="M55.92, L539.2 L539.2 L55.92 L55.92 Z"/>
  </g>
  <g style="fill: #E8E8E8;fill-rule: evenodd;fill-opacity: 1;" id="p2_s430">
    <path d="M55.92, L539.2 L539.2 L55.92 L55.92 Z"/>
  </g>
  <g style="fill: #EFF7EF;fill-rule: evenodd;fill-opacity: 1;" id="p2_s431">
    <path d="M55.92, L539.2 L539.2 L55.92 L55.92 Z"/>
  </g>
  <g style="fill: #EFF7EF;fill-rule: evenodd;fill-opacity: 1;" id="p2_s432">
    <path d="M55.92, L539.2 L539.2 L55.92 L55.92 Z"/>
  </g>
  <g style="fill: #EFF7EF;fill-rule: evenodd;fill-opacity: 1;" id="p2_s433">
    <path d="M55.92, L539.2 L539.2 L55.92 L55.92 Z"/>
  </g>
  <clipPath id="p2_s434" evenodd="true">
    <path d="M56.4,7 L539.6 L539.6 L56.4, L56.4, Z"/>
  </clipPath>
  ......

The svg M or L path command needs both x and y coordinates, but only x coordinate is produced.

Could you tell me if this problem is caused by pdfalto or xpdf and any solution?

philoskim avatar Nov 29 '19 03:11 philoskim

Hello, Any sample that illustrates this issue ?

Aazhar avatar Nov 29 '19 18:11 Aazhar

I have uploaded the sample pdf file that I used in the above example, but I think that every pdf file will produce the similar results.

I spotted the problem here.

 // src/XmlAltoOutputDev.cc

   char *tmp;  // line 6322
   tmp = (char *) malloc(500 * sizeof(char));
   ......

   // The 'sizeof(tmp)' here will always return 8, the size of the pointer itself on 64-bit CPU architecture,
   // not your intended result '500 * sizeof(char)'. 
   snprintf(tmp, sizeof(tmp)," L%g,%g", x1, y1);   // line 6418

   ......
   free(tmp);   // line 6481

My solution is:

 // src/XmlAltoOutputDev.cc

   // char *tmp;  // line 6322
   // tmp = (char *) malloc(500 * sizeof(char));
   char tmp[500 * sizeof(char)];
   ......


   // The 'sizeof(tmp)' here will return your intended result '500 * sizeof(char)'. 
   snprintf(tmp, sizeof(tmp)," L%g,%g", x1, y1);   // line 6418

   ......
   // free(tmp);   // line 6481

philoskim avatar Nov 30 '19 02:11 philoskim

Hello @philoskim !

Thank you very much for the issue and the fix. Indeed this is visible with most of the generated SVG files (either y missing or truncated in the M and L path) and I apologize that we did not spot such an obvious problem in the SVG before.

Regarding the fix, I have to say the current code does often not look very good overall (for example, we are in C++ so we should use new rather than malloc even for the primitive data types) - it is a fork and we reuse most of the existing code. I'll comment on the PR, but one thing, we don't need the sizeof(char) when allocating an array of char (char tmp[N]) because N is the number of elements which is specified, not the memory size in byte.

So thank you again and follow-up in the PR #84

kermitt2 avatar Nov 30 '19 20:11 kermitt2