xdot_gradient_fillcolor: remove dead store to 'r1'
This variable is overwritten on line 648 without having been read in between.
This code appears copy-pasted from `cairo_gradient_fill` without noticing that
this line is unused here.
get_gradient_points: take 'angle' as a double instead of a float
The originating values used to compute an angle were generally doubles and
computation was done in `get_gradient_points` as if this value was a double
(e.g. calling `sin` and `cos`). Treating it consistently as a double squashes a
number of -Wfloat-conversion compiler warnings and improves precision.
gvrender_beziercurve: realign 'filled' parameter type
This appears to have been a mistake, in that all the other `gvrender_*`
functions take an `int` for the `filled` parameter. This does not appear to have
caused any visible effects, as the values passed to this function always fit in
a `boolean`. But the compiler complained that it thought unintended value
truncation was occurring.
This was not configurable and always mapped to the type `double`. There were
also implicit assumptions that `real` and `double` were interchangeable. This
alias is removed to avoid future confusion.
The diff appears large, but it is just removing two #defines of `real` and
propagating their definition (`double`) to all usage sites.
general.h: stop disabling assertions when 'DEBUG' is undefined
The effect of this code was that #including general.h prior to assert.h would
disable assertions in your code. Not #including general.h or #including it after
assert.h would allow assertions to be controlled through the standard `NDEBUG`
mechanism. It is not clear why suppressing assertions like this was ever
desirable.
jacobi: fix assertion typo that overwrote matrix value type
Comparing to the surrounding functions in this file, this was clearly meant to
be checking the type of the matrix not setting it. This was benign as, due to a
separate problem, assertions are disabled in this file, making this a no-op. The
next commit will re-enable assertions, which caused this issue to be discovered
through a compiler error.
fix: use 'find_program' instead of 'find_package' to locate Git in CMake build
The Cygwin CMake CI task recently started stalling after the “Found FLEX” step.
Reordering CMake steps to debug this revealed that it was definitely the
`find_package(Git REQUIRED)` step that was stalling. I have not been able to
root cause what is going on, but some educated guesses:
1. A CMake FindGit.cmake bug? This could certainly cause the described
behavior, but I cannot find anything online to corroborate this and cannot
see anything in the upstream history of this file¹ that looks like a fix
for something like this.
2. Installing a version of CMake via Choco and then running it via Cygwin
causes some bad Windows/UNIX interaction? Perhaps, but I cannot find any
corroborating evidence for this either. Also, I cannot guess why such a
thing would _only_ cause problems in FindGit.cmake.
3. FindGit.cmake is picking up a Visual Studio shim for Git that responds in a
way that confuses the `find_package` check? I found vague hints about a
shim like this online, but I still cannot explain why this would cause a
stall.
A work around for this is to use the less nuanced `find_program`. This has no
negative impact on the CMake setup because (1) all we are attempting to detect
is the `git` binary itself and (2) the CMake files never call `git` directly
anyway; we are performing this detection to avoid gen_version.py failing
cryptically.
Note that it is not possible to use `REQUIRED` in this `find_program` directive
because this was only added in CMake 3.18 and we currently only require CMake
3.1.
This also re-enables the Cygwin CMake CI task now that it no longer stalls.
This is part of a goal to reduce release artifacts to ≤50 files so the list of
files is not truncated in Gitlab pages. MD5 is an obsolete checksum format that
has been deprecated for many years. This commit removes these checksums from
being produced in the deployment flow, leaving SHA256 checksums as the only
option. This is consistent with Gitlab’s Generic Package Registry that provides
an alternative way of seeing file checksums and also only offers SHA256.
This change reduces the number of release artifacts from 56 to 46.
Now that nothing is written to the sibling `debug` and `source` directories,
there does not seem to be an advantage to retaining this level in the hierarchy.
rename CMake-produced DEB t *-cmake to disambiguate
Now that the Autotools-produced DEBs are consolidated into tarballs, someone
looking at a flat list of all the release artifacts could incorrectly conclude
that Graphviz-${GV_VERSION}-Linux.deb is an alternative complete package to the
tarballs. Renaming this makes it clearer that it originates from the CMake build
system and is incomplete.
Note that this also removes the `os` hierarchy in the packaging directories for
these artifacts. All DEB artifacts are now produced without any
`os`/`debug`/`source` subdirectory.
This drops the total number of Graphviz release artifacts from 63 to 58. This is
still not below the 50 artifact ceiling we need to hit, but this is progress.
Note that this removes the `os`/`debug`/`source` hierarchy in the packaging
directories for these artifacts, now that all three of these are packaged into a
single tarball. The `os` hierarchy remains for the CMake-produced DEB for these
platforms.
This drops the total number of Graphviz release artifacts from 168 to 63. This
is still not below the 50 artifact ceiling we need to hit, but this is
significant progress.
rename CMake-produced RPM to *-cmake to disambiguate
Now that the Autotools-produced RPMs are consolidated into tarballs, someone
looking at a flat list of all the release artifacts could incorrectly conclude
that Graphviz-${GV_VERSION}-Linux.rpm is an alternative complete package to the
tarballs. Renaming this makes it clearer that it originates from the CMake build
system and is incomplete.
Note that this also removes the `os` hierarchy in the packaging directories for
these artifacts. All RPM artifacts are now produced without any
`os`/`debug`/`source` subdirectory.
This drops the total number of Graphviz release artifacts from 176 to 168. This
is still not below the 50 artifact ceiling we need to hit, but this is progress.
Note that this removes the `os`/`debug`/`source` hierarchy in the packaging
directories for these artifacts, now that all three of these are packaged into a
single tarball. The `os` hierarchy remains for the CMake-produced RPM for these
platforms.
This drops the total number of Graphviz release artifacts from 340 to 176. This
is still not below the 50 artifact ceiling we need to hit, but this is
significant progress.
This #define was not conditional (it was always set to `free`) and was not used
consistently (there were still direct calls to `free` in some places that should
have used this macro), incorrectly giving the impression that it could be safely
swapped for another function.
gts_surface_foreach_edge: fix mismatch of calling convention in callback
The compiler said about this code:
delaunay.c: In function ‘edgeStats’:
delaunay.c:245:34: warning: cast between incompatible function types from
‘void (*)(GtsSegment *, estats *)’ {aka ‘void (*)(struct _GtsSegment *,
struct <anonymous> *)’} to ‘gint (*)(void *, void *)’ {aka
‘int (*)(void *, void *)’} [-Wcast-function-type]
gts_surface_foreach_edge (s, (GtsFunc) cnt_edge, sp);
^
This is not quite as bad as the previous instance of this fixed. However, it
seems to have been relying on a coincident return value. E.g. on x86-64 this
code relies on 0 ending up in `RAX` at the end of the callback to indicate
iteration should continue, despite `cnt_edge` having no declared return value.
delaunay_remove_holes: fix mismatch of calling convention in callback
The compiler said about this code:
delaunay.c: In function ‘delaunay_remove_holes’:
delaunay.c:45:9: warning: cast between incompatible function types from
‘gboolean (*)(GtsTriangle *)’ {aka ‘int (*)(struct _GtsTriangle *)’}
to ‘gint (*)(void *, void *)’ {aka ‘int (*)(void *, void *)’}
[-Wcast-function-type]
(GtsFunc) triangle_is_hole, NULL);
This warning is not spurious. In particular, the mismatch in the number of
arguments passed to the callback means the callback function has a different
calling convention than expected by the code calling it. The result of this can
be stack corruption or incorrect interpretation of function arguments.
In practice, all major native calling conventions use registers for both
function types, so this is somewhat benign. However, this likely caused problems
on stack-based environments like WASM and JITs.
projects / graphviz / log