simon-git: puzzles (main): Simon Tatham
Commits to Tartarus hosted VCS
tartarus-commits at lists.tartarus.org
Thu Aug 15 18:23:06 BST 2024
TL;DR:
f379130 Refine drawing API semantics to pass `drawing *` instead of `void *`
4149f2c Add comment pointing to the rationale for versioning the drawing API.
989df5d Add draw_polygon_fallback() for platforms without a native polygon fill.
a8b544d Add USE_DRAW_POLYGON_FALLBACK build option for testing.
Repository: https://git.tartarus.org/simon/puzzles.git
On the web: https://git.tartarus.org/?p=simon/puzzles.git
Branch updated: main
Committer: Simon Tatham <anakin at pobox.com>
Date: 2024-08-15 18:23:06
commit f37913002a62dfe701c9a89e7532202c6587bff3
web diff https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=f37913002a62dfe701c9a89e7532202c6587bff3;hp=a993fd45ebe67946e165cbe579bc3e3a596dcded
Author: Franklin Wei <franklin at rockbox.org>
Date: Sun Aug 11 19:28:35 2024 -0400
Refine drawing API semantics to pass `drawing *` instead of `void *`
This changes the drawing API so that implementations receive a
`drawing *` pointer with each call, instead of a `void *` pointer as
they did previously. The `void *` context pointer has been moved to be
a member of the `drawing` structure (which has been made public), from
which it can be retrieved via the new `GET_HANDLE_AS_TYPE()` macro. To
signal this breaking change to downstream front end authors, I've
added a version number to the `drawing_api` struct, which will
hopefully force them to notice.
The motivation for this change is the upcoming introduction of a
draw_polygon_fallback() function, which will use a series of calls to
draw_line() to perform software polygon rasterization on platforms
without a native polygon fill primitive. This function is fairly
large, so I desired that it not be included in the binary
distribution, except on platforms which require it (e.g. my Rockbox
port). One way to achieve this is via link-time optimization (LTO,
a.k.a. "interprocedural optimization"/IPO), so that the code is
unconditionally compiled (preventing bit-rot) but only included in the
linked executable if it is actually referenced from elsewhere.
Practically, this precludes the otherwise straightforward route of
including a run-time check of the `draw_polygon` pointer in the
drawing.c middleware. Instead, Simon recommended that a front end be
able to set its `draw_polygon` field to point to
draw_polygon_fallback(). However, the old drawing API's semantics of
passing a `void *` pointer prevented this from working in practice,
since draw_polygon_fallback(), implemented in middleware, would not be
able to perform any drawing operations without a `drawing *` pointer;
with the new API, this restriction is removed, clearing the way for
that function's introduction.
This is a breaking change for front ends, which must update their
implementations of the drawing API to conform. The migration process
is fairly straightforward: every drawing API function which previously
took a `void *` context pointer should be updated to take a `drawing *`
pointer in its place. Then, where each such function would have
previously casted the `void *` pointer to a meaningful type, they now
instead retrieve the context pointer from the `handle` field of the
`drawing` structure. To make this transition easier, the
`GET_HANDLE_AS_TYPE()` macro is introduced to wrap the context pointer
retrieval (see below for usage).
As an example, an old drawing API function implementation would have
looked like this:
void frontend_draw_func(void *handle, ...)
{
frontend *fe = (frontend *)handle;
/* do stuff with fe */
}
After this change, that function would be rewritten as:
void frontend_draw_func(drawing *dr, ...)
{
frontend *fe = GET_HANDLE_AS_TYPE(dr, frontend);
/* do stuff with fe */
}
I have already made these changes to all the in-tree front ends, but
out-of-tree front ends will need to follow the procedure outlined
above.
Simon pointed out that changing the drawing API function pointer
signatures to take `drawing *` instead of `void *` results only in a
compiler warning, not an outright error. Thus, I've introduced a
version field to the beginning of the `drawing_api` struct, which will
cause a compilation error and hopefully force front ends to notice
this. This field should be set to 1 for now. Going forward, it will
provide a clear means of communicating future breaking API changes.
devel.but | 128 +++++++++++++++++++++++++---------------
drawing.c | 196 +++++++++++++++++++++++++++++++++++++------------------------
emcc.c | 35 +++++------
fuzzpuzz.c | 2 +-
gtk.c | 89 ++++++++++++++--------------
nestedvm.c | 51 ++++++++--------
nullfe.c | 1 -
osx.m | 63 ++++++++++----------
ps.c | 65 ++++++++++----------
puzzles.h | 82 ++++++++++++++++++--------
windows.c | 89 ++++++++++++++--------------
11 files changed, 459 insertions(+), 342 deletions(-)
commit 4149f2cb9c93333aa2456eaac57406a8c1798707
web diff https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=4149f2cb9c93333aa2456eaac57406a8c1798707;hp=f37913002a62dfe701c9a89e7532202c6587bff3
Author: Franklin Wei <franklin at rockbox.org>
Date: Wed Aug 14 20:28:57 2024 -0400
Add comment pointing to the rationale for versioning the drawing API.
I'm guessing that front end authors might look here to investigate a
build failure due to the recent changes to the drawing API.
(This unfortunately needs to be a separate commit from its parent --
otherwise it would need to reference its own hash!)
puzzles.h | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
commit 989df5d2bf79eaee2ee4fb21d9be4341b60569c7
web diff https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=989df5d2bf79eaee2ee4fb21d9be4341b60569c7;hp=4149f2cb9c93333aa2456eaac57406a8c1798707
Author: Franklin Wei <franklin at rockbox.org>
Date: Sun Aug 11 20:52:52 2024 -0400
Add draw_polygon_fallback() for platforms without a native polygon fill.
This adds a portable, scanline-based polygon filling algorithm, which
fills a polygon by drawing a collection of adjacent horizontal lines.
This change is motivated by the Rockbox port's current lack of a true
polygon fill capability. Until now, it attempted to approximate a
polygon fill by performing a series of triangle fills, but this worked
reliably only for convex polygons. I originally considered making this
new rasterizer part of the Rockbox front end itself, but I ultimately
decided that it made more sense to include it here, in the Puzzles
distribution, where other platforms may benefit from it in the future.
No in-tree front ends use this new function quite yet, but I plan to
follow this commit with a compile-time option to force front ends to
use it for testing.
This new polygon drawing code also comes with its own standalone
driver code to test it out in isolation. This code currently relies on
SDL 2.0 to present a GUI window to the user, which unfortunately adds
a build-time dependency. To lessen the impact of this change, this
program is gated behind a CMake build option. To use it, run:
$ cmake -DBUILD_SDL_PROGRAMS=true
CMakeLists.txt | 10 +-
cmake/setup.cmake | 10 +-
devel.but | 38 +++++++
draw-poly.c | 302 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
nullfe.c | 2 +
puzzles.h | 2 +
6 files changed, 359 insertions(+), 5 deletions(-)
commit a8b544d2aa9d22a2db0af90733a08970c3dcf28e
web diff https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=a8b544d2aa9d22a2db0af90733a08970c3dcf28e;hp=989df5d2bf79eaee2ee4fb21d9be4341b60569c7
Author: Franklin Wei <franklin at rockbox.org>
Date: Wed Aug 14 17:53:41 2024 -0400
Add USE_DRAW_POLYGON_FALLBACK build option for testing.
This new option, when enabled, forces the in-tree front ends
(Emcscripten, GTK, NestedVM, OS X, and Windows) to use the recently
introduced draw_polygon_fallback() in place of their native
draw_poly(). This will enable easy testing of this function in the
future.
This new option is off by default. To enable it, run CMake as:
$ cmake -DUSE_DRAW_POLYGON_FALLBACK=on
Note that I did _not_ update the Postscript frontend (ps.c) to use
this new option, as I don't think draw_polygon_fallback() would work
at all in Postscript, where the drawing units are no longer guaranteed
to be pixels. The envisioned use case for this option is a developer
testing changes to this function for sanity and/or performance, which
I only foresee happening on a standard GUI front end.
cmake/setup.cmake | 5 +++++
emcc.c | 4 ++++
gtk.c | 4 ++++
nestedvm.c | 4 ++++
osx.m | 4 ++++
windows.c | 4 ++++
6 files changed, 25 insertions(+)
More information about the tartarus-commits
mailing list