#836 libgd: Fixes for minor issues introduced with patch 833 (dashtype support)

[Sethur]

Hi @sfeam,

While testing the plot area clipping implementation you proposed I came across some minor issues that have been introduced with my patch #833 (libgd oversampling => support for dashtype and better rendering).

I have created patches for these issues as well as accompanying documentation. I used LLMs to help with coding and documentation and thoroughly reviewed everything:

1. fix-libgd-thick-line-artifacts.mbox/md: There was an issue with the join discs being drawn when the angles between line segments were too large. Depending on line thickness and sampling rate, these joins were not drawn for very thick lines when they should have been, introducing visual artifacts (missing white pixels) like in the example image. The patch fixes this issue by making the condition on when to draw these joins depend on the line thickness.

2. fix-libgd-custom-and-dashtypes-greater5.mbox/md:
   2a) The pattern of the numeric dashtypes greater than 5 (i.e., beginning with dt 6) was wrong and not matching the pattern repetition of the cairo terminals. Before this fix, the dash patterns were repeating, but the solid line at position 6 was not being used. I changed this now to imitate the behavior of the cairo, but upon testing this, I found that the postscript terminal uses a totally different assortment of dash patterns, so maybe you want to adapt this to your liking for libgd.
   2b) The dashes of the custom dash patterns (like dt "._") were too long compared to cairo. This was originally introduced because with the empirical_scale of 0.25 used by cairo, small dash/gap-patterns would have been merged by libgd for small line widths. This patch fixes this issue and sets the length of these custom patterns to match cairo.

3. fix-libgd-screen-boundary-rounding.mbox/md: While staring at demo output, I found that, for example transparent_solids.dem, that draws a black rectangular thin border from screen 0.0 to screen 1.0, was missing the bottom and the right border line with the new libgd rendering approach. It turned out that the coordinate system with the new oversampling approach translated screen 1.0 to a coordinate outside of the image, which resulted in these lines not being drawn. The fix for this was a little more involved, because in the oversampled coordinate space, it matters whether you want to draw a line from pixel center to pixel center or fill an area when coordinates are converted back to image space.

[Sethur]

PS: I just found that fix-libgd-screen-boundary-rounding.md containes some references to a previous approach where I tried clamping the coordinates (which also worked but wasn't as clean). I have fixed that in the doc below.

[Ethan Merritt]

Sorry for the delay; I've been dealing with other things.
These patches look very LLM-ish. Lots of code that doesn't do much and accompanying text that does not really match what's in the code. The first two fix real issues, so thanks for those.

fix-libgd-thick-line-artifacts
I replaced this with a 1-line fix.
3 other lines introduce an unnecessary global constant that can be in-lined.
The md file is basically nonsense.

fix-libgd-custom-and-dashtypes-greater5
The fix to include solid lines in the cycle was also a 1-liner. However the patch made the change in the wrong place, which made other places unnecessarily complicated and hard to understand.
I applied the relocated fix.

The md file seems to indicate there are other fixes, but I am dubious about their quality.
If there really is a need to "avoid dash-walker stalls at element boundaries" then I think a simpler, more obvious, fix should be found. Why isn't this just
if (step < 0.001) break;
Anyhow, magic numbers are bad. I don't like any of the tests in this section of code that involve 0.01 or 0.001 as opaque constants, including the loop end condition.

fix-libgd-screen-boundary-rounding.mbox
Rejected.
Redefining the coordinate system to center each pixel on an integral [x,y] is counter-productive. For pixel size > 1 that introduces a requirement to clip every single border pixel.
Furthermore it would be a different convention from every other terminal type I can think of.

[Sethur]

I think you might be a bit harsh on the LLM here. As I said before, I completely agree that blind trust into LLM output is very bad, and using it without any programming or computer science experience is likely going to end very badly, at least with the current models and especially for science software where the accuracy of the output matters. I also agree that the way the LLM was describing the patches was often not the most elegant prose, but to say that it is complete nonsense might be a stretch. Also, I have quite extensive experience in programming, especially in C/C++, and am just using the LLMs because I would otherwise have needed to spend excessive amounts of time to acquire enough knowledge about the gnuplot codebase to be able to meaningfully contribute here. If it would help you to trust my input a bit more I could e-mail you some credentials.

Regarding your comments to the patches, I can straight away say that, for example, in fix-libgd-thick-line-artifacts, your dismissal of the global constant (and I think you are referring to PNG_HARD_OVERLAP) does not make much sense to me. The condition on which the join disk is to be drawn is clearly dependent on the chosen overlap of the line segments and if you in-line this, a change of the overlap would then not automatically translate to the join disks being drawn more or less often (as they should).

I am currently traveling, but I will go over the patches once again and rewrite the markdown commentaries in my own words.

Since you rejected fix-libgd-screen-boundary-rounding, maybe you have some better ideas about how to tackle the issue with screen 1.0 ending up one pixel outside the drawable area with the current oversampled libgd implementation.

EDIT: less often => more or less often (overlap could also be reduced)

[Ethan Merritt]

I found a glitch in the clipping behavior after git am < fix-libgd-screen-boundary-rounding.mbox. I found this by rotating the view interactively; the glitch occurs when the polygon borders touch or cross the left edge of the canvas.
Reproducing script attached.

• version 6.0.4 - correct plot
• git version prior to commit - the program hangs.
• git version after commit - glitch in polygon border line clipping (see attached output).

However after a bit of debugging I think this is due to a problem introduced in the original gd clipping commit; I just didn't notice it until now. It comes from trying to clip vectors in the terminal code rather than in the core code. The problem is that the terminal API uses (unsigned int) coordinates. If the core passes in a negative number it is mis-interpreted as a very large positive number. This is OK for points, since both the negative value and the large positive value are out of range. But you cannot use the incorrect positive value for interpolation to find a new endpoint for a clipped vector.

I guess I did not look at the original gd.trm modifications closely enough to notice this. I was thinking only in terms of clipping an area within a larger canvas as a rectangle; I didn't realize that the new code was trying to clip vectors before drawing them onto the larger canvas.

A very quick trial with the attached patch suggests it may be possible to fix this by redefining the set of macros GD_PX() GD_Y() ... but I did not test extensively. It might affect either too many or too few places where the macros are used.

[Sethur]

Thanks, I will have a look at this and check if the patch is sufficient. I could potentially integrate this in a patch for another issue with the coordinate system of gd.trm I stumbled upon:

Internally, libgd is interpreting integer x- and y-coordinates as pixel centers (cf. Wu anti-aliasing at line no. 4427 in libgd/gd.c), while in gd.trm, they are currently interpreted as the top-left pixel edges. This means that, e.g., plotting a line from x=10.0, y=10.0 to x=20.0, y=10.0 with the new SDF rasterizer will plot the line directly on the pixel boundaries, meaning that the rasterizer effectively produces a line that is 2 pixels wide but has 50% coverage (i.e. is not fully opaque) in both rows beloning to the line.

Previously, this was not a problem since lw=1 lines were not drawn with the internal SDF rasterizer but still delegated to libgd for performance reasons.

I realized this while trying to find a nicer solution for the custom dash patterns issue addressed in fix-libgd-custom-and-dashtypes-greater5 by this code:

+   if (cap_end != PNG_CAP_HARD) {
+       if (ix2 != ix1 || iy2 != iy1) {
+       if (abs(ix2 - ix1) >= abs(iy2 - iy1)) {
+           if (ix2 > ix1) ix2--;
+           else ix2++;
+       } else {
+           if (iy2 > iy1) iy2--;
+           else iy2++;
+       }
+       }
+   }
+

I have checked this thoroughly and looked into the libgd code as well to confirm that going back one pixel on the dominant axis is conformant to how AA'd lines are drawn in libgd's gdImageLine(). I am also convinced that excluding the last pixel in the gdImageLine() call is the proper solution here to achieve, on average, the correct total line length for segments with endcaps (including dashes), but I also realize that this solution is dependent on the inner workings of gdImageLine() and does not look so nice.

An alternative approach would be to use the SDF rasterizer for all dashed lines. Since the SDF rasterizer properly reduces the coverage of pixels that are near the end of a capped line segment (i.e., the end of a dash or the total end of a line), the issue that very short dash segments get merged with one another is not an issue here. However, with the current implementation, the SDF rasterizer would:

a) Make straight 1px-wide lines on pixel borders effectively two pixels wide and half-transparent

b) Take about 1.7 times longer to render a dashed line than the gdImageLine() implementation.

As I said above, I am working on an elegant fix to mitigate these issues in the SDF rasterizer, so, if you prefer, we could use it to render dashed lines, which would avoid using the patch mentioned above.

To better visualize why we even need the fix I have attached the output of the dashtypes.dem script using (two plots) in a 2x2 montage using:

1) terminal png with the alternative approach of using the SDF rasterizer for all dashed lines
2) terminal pngwith the original fix (code above) using gdImageLine() for lines with lw = 1
3) terminal pngwith no fixes at all (you can clearly see the merged dashes in the custom dash patterns, especially the first one)
4) terminal pngcairoas the gold standard

Regarding your comments about the guard in the dash walker: This is really necessary and if (step < 0.001) break; is not sufficient here as the stall can happen anywhere within a line segment and this would then skip all remaining dashes and gaps that might still occur in the current segment (and also lead to the same issue on the next iteration of the loop). We need to safely advance PNG_dash_pos to the next element (dash or gap), which is exactly what the patch does. Without this patch, I had infinite loops when I set the empirical_scale factor to 0.35 for testing. You are right that just using 0.001 as a magic number for checking the minimum step size is not optimal. I have introduced a constant to make this cleaner. We cannot just use something like DBL_EPSILONhere as this would not guarantee that PNG_dash_pos can be safely advanced to the next double value. We need something that is large enough to advance PNG_dash_pos value and small enough to make not visual difference. 0.001seems like a good compromise to me.

I have rebased the custom dashlength patch onto the current master branch (including your partial adoption of the content). The patch includes both the original fix and the optional approach that uses the SDF rasterizer for all dashed lines.

I would still opt for using the original patch because of the performance advantage but I would understand if you would prefer the alternative approach.

I know that you do not like LLM speech (neither do I), so this post is entirely my own. I have, however, still attached a documentation for the new patch that was 80% generated with an LLM, but word-by-word reviewed by me. Plus I also changed some parts and added some paragraphs. I think the doc really helps to understand the patches.

I will post here again if I have a patch for the libgd coordinate system, which would then also include a patch for the int <=> unsigned int issue you have posted above. I will also try to find a nicer solution for the screen 1.0issue that you rejected.

EDIT: Typos in the first paragraph.

[Sethur]

I have checked your patch for the int <=> unsigned int issue and I think it is sufficient to fix the issue. I still have to go through all of the test output, but what I have already seen so far looks good.

I have also already implemented a fix for the pixel-center based coordinate system described above.

If you want to have a look, please find a patch attached (it's quite small).

The new coordinate system approach fixes the issue with 1px-lines being too wide with the SDF rasterizer and also better matches the cairo output for other line thicknesses (see example dashtypes.dem plots, this time with the new coordinate system in the top left plot).

[Ethan Merritt]

28-May-2026

Attached here is a mbox file containing a series of commits on top of current git tip.
I hope I have not become confused about the order and applicability of the various patches in this thread. Other than moving a couple of declarations to avoid compiler warnings, I have used your patches as they were.

The final commit in this series is a fix for the "screen 1.0" issue. This repairs an error dating back to way before any of your work. The quantity being reported in term->ymax was incorrectly loaded with the height (number of pixels on y). Since terminal coordinates run from 0 to size-1, ymax is really ysize - 1. I fixed this to load term-ymax correctly and replaced png_state.height with term->ymax everywhere. Seems to work.

If I have misinterpreted your intended patch order or missed something, please let me know or send a corrected commit series.

Question: I originally modified gd.trm to draw thick lines with pen type gdBrushed because the library routines did a poor job of drawing thick lines with a normal pen. Now that you have implemented a different thick line mechanism, can we get rid of all the PNG_brush + gdBrushed code? If so, that will probably be both faster and more readable.

[Ethan Merritt]

Update: I broke something, probably with that last commit. In nonlinear3.dem the images become fragmented.

[Ethan Merritt]

Never mind. The revised macros PY(y) etc needed another level of wrapping parentheses.

[Sethur]

Have you inckuded the revised macros in the mbox posted above?

Sorry for the late reaction, I am traveling again for the week and don't have access to my dev environment until I come back.

I will have a look at it when I return.

[Ethan Merritt]

No. But this set includes the revised macros and also the removal of gdBrushed pen type.

Note: Perhaps something has changed since this empirical correction was introduced. To my eyes it would better be 0.5 rather than 0.25

         /* Custom string patterns (".-_") have DSCALE=10 baked in;

          * apply empirical_scale=0.25 to match pngcairo visual output.*/
         scale = 0.25 * png_state.linewidth_f * PNG_dashlength_factor;