diff --git a/contrib/bash_compl/_rgbgfx.bash b/contrib/bash_compl/_rgbgfx.bash index 9d49dd7e..cf412a53 100755 --- a/contrib/bash_compl/_rgbgfx.bash +++ b/contrib/bash_compl/_rgbgfx.bash @@ -36,6 +36,7 @@ _rgbgfx_completions() { [s]="palette-size:unk" [t]="tilemap:glob-*.tilemap" [T]="auto-tilemap:normal" + [W]="warning:warning" [x]="trim-end:unk" ) # Parse command-line up to current word @@ -152,6 +153,14 @@ _rgbgfx_completions() { case "$state" in unk) # Return with no replies: no idea what to complete! ;; + warning) + mapfile -t COMPREPLY < <(compgen -W " + embedded + trim-nonempty + all + everything + error" -P "${cur_word:0:$optlen}" -- "${cur_word:$optlen}") + ;; normal) # Acts like a glob... state="glob-*.png" ;& diff --git a/contrib/zsh_compl/_rgbgfx b/contrib/zsh_compl/_rgbgfx index 91f474c6..4c89d8c1 100644 --- a/contrib/zsh_compl/_rgbgfx +++ b/contrib/zsh_compl/_rgbgfx @@ -9,6 +9,20 @@ _depths() { _describe 'bit depth' depths } +_rgbgfx_warnings() { + local warnings=( + 'error:Turn all warnings into errors' + + 'all:Enable most warning messages' + 'everything:Enable literally everything' + + 'embedded:Warn when using embedded PLTE without "-c embedded"' + 'trim-nonempty:Warn when "-x" trims nonempty tiles' + ) + # TODO: handle `no-` and `error=` somehow? + _describe warning warnings +} + local args=( # Arguments are listed here in the same order as in the manual, except for the version and help '(- : * options)'{-V,--version}'[Print version number and exit]' @@ -23,6 +37,7 @@ local args=( '(-t --tilemap -T --auto-tilemap)'{-T,--auto-tilemap}'[Shortcut for -t .tilemap]' '(-u --unique-tiles)'{-u,--unique-tiles}'[Eliminate redundant tiles]' {-v,--verbose}'[Enable verbose output]' + -w'[Disable all warnings]' '(-X --mirror-x)'{-X,--mirror-x}'[Eliminate horizontally mirrored tiles from output]' '(-Y --mirror-y)'{-Y,--mirror-y}'[Eliminate vertically mirrored tiles from output]' '(-Z --columns)'{-Z,--columns}'[Read the image in column-major order]' @@ -42,6 +57,7 @@ local args=( '(-r --reverse)'{-r,--reverse}'+[Yield an image from binary data]:image width (in tiles):' '(-s --palette-size)'{-s,--palette-size}'+[Limit palette size]:palette size:' '(-t --tilemap -T --auto-tilemap)'{-t,--tilemap}'+[Generate a map of tile indices]:tilemap file:_files' + '(-W --warning)'{-W,--warning}'+[Toggle warning flags]:warning flag:_rgbgfx_warnings' '(-x --trim-end)'{-x,--trim-end}'+[Trim end of output by this many tiles]:tile count:' ":input png file:_files -g '*.png'" diff --git a/include/gfx/warning.hpp b/include/gfx/warning.hpp index d40e989d..af6476d1 100644 --- a/include/gfx/warning.hpp +++ b/include/gfx/warning.hpp @@ -3,6 +3,29 @@ #ifndef RGBDS_GFX_WARNING_HPP #define RGBDS_GFX_WARNING_HPP +#include "diagnostics.hpp" + +enum WarningLevel { + LEVEL_DEFAULT, // Warnings that are enabled by default + LEVEL_ALL, // Warnings that probably indicate an error + LEVEL_EVERYTHING, // Literally every warning +}; + +enum WarningID { + WARNING_EMBEDDED, // Using an embedded PNG palette without '-c embedded' + WARNING_TRIM_NONEMPTY, // '-x' trims nonempty tiles + + NB_PLAIN_WARNINGS, + + NB_WARNINGS = NB_PLAIN_WARNINGS, +}; + +extern Diagnostics warnings; + +// Warns the user about problems that don't prevent valid graphics conversion +[[gnu::format(printf, 2, 3)]] +void warning(WarningID id, char const *fmt, ...); + // Prints the error count, and exits with failure [[noreturn]] void giveUp(); diff --git a/man/rgbgfx.1 b/man/rgbgfx.1 index 3c87186f..b86c5861 100644 --- a/man/rgbgfx.1 +++ b/man/rgbgfx.1 @@ -10,7 +10,7 @@ .Nd Game Boy graphics converter .Sh SYNOPSIS .Nm -.Op Fl CmhOuVXYZ +.Op Fl CmhOuVwXYZ .Op Fl v Op Fl v No ... .Op Fl a Ar attrmap | Fl A .Op Fl b Ar base_ids @@ -27,6 +27,7 @@ .Op Fl r Ar width .Op Fl s Ar nb_colors .Op Fl t Ar tilemap | Fl T +.Op Fl W Ar warning .Op Fl x Ar quantity .Ar file .Sh DESCRIPTION @@ -393,6 +394,17 @@ Some internal debug printing is enabled. The verbosity level does not go past 6. .Pp Note that verbose output is only intended to be consumed by humans, and may change without notice between RGBDS releases; relying on those for scripts is not advised. +.It Fl W Ar warning , Fl \-warning Ar warning +Set warning flag +.Ar warning . +A warning message will be printed if +.Ar warning +is an unknown warning flag. +See the +.Sx DIAGNOSTICS +section for a list of warnings. +.It Fl w +Disable all warning output, even when turned into errors. .It Fl X , Fl \-mirror-x Deduplicate tiles that are horizontally symmetrical mirror images of each other across the X axis. Implies @@ -708,6 +720,68 @@ assume that tiles were not deduplicated, and should be laid out in the order the .Nm assumes that no tiles were mirrored. .El +.Sh DIAGNOSTICS +Warnings are diagnostic messages that indicate possibly erroneous behavior that does not necessarily compromise the conversion process. +The following options alter the way warnings are processed. +.Bl -tag -width Ds +.It Fl Werror +Make all warnings into errors. +This can be negated as +.Fl Wno-error +to prevent turning all warnings into errors. +.It Fl Werror= +Make the specified warning or meta warning into an error. +A warning's name is appended +.Pq example: Fl Werror=embedded , +and this warning is implicitly enabled and turned into an error. +This can be negated as +.Fl Wno-error= +to prevent turning a specified warning into an error, even if +.Fl Werror +is in effect. +.El +.Pp +The following warnings are +.Dq meta +warnings, that enable a collection of other warnings. +If a specific warning is toggled via a meta flag and a specific one, the more specific one takes priority. +The position on the command-line acts as a tie breaker, the last one taking effect. +.Bl -tag -width Ds +.It Fl Wall +This enables warnings that are likely to indicate an error or undesired behavior, and that can easily be fixed. +.It Fl Weverything +Enables literally every warning. +.El +.Pp +The following warnings are actual warning flags; with each description, the corresponding warning flag is included. +Note that each of these flag also has a negation (for example, +.Fl Wtrim-nonempty +enables the warning that +.Fl Wno-trim-nonempty +disables; and +.Fl Wall +enables every warning that +.Fl Wno-all +disables). +Only the non-default flag is listed here. +Ignoring the +.Dq no- +prefix, entries are listed alphabetically. +.Bl -tag -width Ds +.It Fl Wembedded +Warn when a generated palette is sorted according to the input PNG's embedded palette but +.Fl c Cm embedded +was not provided. +This warning is enabled by +.Fl Weverything . +.It Fl Wtrim-nonempty +Warn when +.Fl x +trims a nonempty tile. +An "empty" tile uses entirely color 0 of its palette. +This warning is enabled by +.Fl Wall . +.El .Sh EXAMPLES The following will only validate the .Ql tileset.png diff --git a/src/gfx/main.cpp b/src/gfx/main.cpp index 903eb6ab..fb4ec63e 100644 --- a/src/gfx/main.cpp +++ b/src/gfx/main.cpp @@ -52,7 +52,7 @@ void Options::verbosePrint(uint8_t level, char const *fmt, ...) const { } // Short options -static char const *optstring = "-Aa:B:b:Cc:d:hi:L:l:mN:n:Oo:Pp:Qq:r:s:Tt:U:uVvXx:YZ"; +static char const *optstring = "-Aa:B:b:Cc:d:hi:L:l:mN:n:Oo:Pp:Qq:r:s:Tt:U:uVvW:wXx:YZ"; // Equivalent long options // Please keep in the same order as short opts. @@ -90,6 +90,7 @@ static option const longopts[] = { {"unique-tiles", no_argument, nullptr, 'u'}, {"version", no_argument, nullptr, 'V'}, {"verbose", no_argument, nullptr, 'v'}, + {"warning", required_argument, nullptr, 'W'}, {"mirror-x", no_argument, nullptr, 'X'}, {"trim-end", required_argument, nullptr, 'x'}, {"mirror-y", no_argument, nullptr, 'Y'}, @@ -558,6 +559,12 @@ static char *parseArgv(int argc, char *argv[]) { } break; // LCOV_EXCL_STOP + case 'W': + warnings.processWarningFlag(musl_optarg); + break; + case 'w': + warnings.state.warningsEnabled = false; + break; case 'x': options.trim = parseNumber(arg, "Number of tiles to trim", 0); if (*arg != '\0') { diff --git a/src/gfx/process.cpp b/src/gfx/process.cpp index 81219312..711e665c 100644 --- a/src/gfx/process.cpp +++ b/src/gfx/process.cpp @@ -615,6 +615,10 @@ static std::tuple, std::vector> sortGrayscale(palettes, png.getColors().raw()); } else if (auto [embPalSize, embPalRGB, embPalAlphaSize, embPalAlpha] = png.getEmbeddedPal(); embPalRGB != nullptr) { + warning( + WARNING_EMBEDDED, + "Sorting palette colors by PNG's embedded PLTE chunk without '-c/--colors embedded'" + ); sortIndexed(palettes, embPalSize, embPalRGB, embPalAlphaSize, embPalAlpha); } else if (png.isSuitableForGrayscale()) { sortGrayscale(palettes, png.getColors().raw()); @@ -870,32 +874,41 @@ static void outputUnoptimizedTileData( uint16_t widthTiles = options.inputSlice.width ? options.inputSlice.width : png.getWidth() / 8; uint16_t heightTiles = options.inputSlice.height ? options.inputSlice.height : png.getHeight() / 8; - uint64_t remainingTiles = widthTiles * heightTiles; - if (remainingTiles <= options.trim) { - return; - } - remainingTiles -= options.trim; + uint64_t nbTiles = widthTiles * heightTiles; + uint64_t nbKeptTiles = nbTiles > options.trim ? nbTiles - options.trim : 0; + uint64_t tileIdx = 0; for (auto [tile, attr] : zip(png.visitAsTiles(), attrmap)) { // Do not emit fully-background tiles. if (!attr.isBackgroundTile()) { // If the tile is fully transparent, this defaults to palette 0. Palette const &palette = palettes[attr.getPalID(mappings)]; + + bool empty = true; for (uint32_t y = 0; y < 8; ++y) { uint16_t bitplanes = TileData::rowBitplanes(tile, palette, y); - output->sputc(bitplanes & 0xFF); - if (options.bitDepth == 2) { - output->sputc(bitplanes >> 8); + if (bitplanes != 0) { + empty = false; + } + if (tileIdx < nbKeptTiles) { + output->sputc(bitplanes & 0xFF); + if (options.bitDepth == 2) { + output->sputc(bitplanes >> 8); + } } } - } - --remainingTiles; - if (remainingTiles == 0) { - break; + if (!empty && tileIdx >= nbKeptTiles) { + warning( + WARNING_TRIM_NONEMPTY, + "Trimming a nonempty tile (configure with '-x/--trim-end'" + ); + break; // Don't repeat the warning for subsequent tiles + } } + ++tileIdx; } - assume(remainingTiles == 0); + assume(nbKeptTiles <= tileIdx && tileIdx <= nbTiles); } static void outputUnoptimizedMaps( diff --git a/src/gfx/warning.cpp b/src/gfx/warning.cpp index d161e773..d7c6aaf8 100644 --- a/src/gfx/warning.cpp +++ b/src/gfx/warning.cpp @@ -10,6 +10,21 @@ static uintmax_t nbErrors; +// clang-format off: nested initializers +Diagnostics warnings = { + .metaWarnings = { + {"all", LEVEL_ALL }, + {"everything", LEVEL_EVERYTHING}, + }, + .warningFlags = { + {"embedded", LEVEL_EVERYTHING}, + {"trim-nonempty", LEVEL_ALL }, + }, + .paramWarnings = {}, + .state = DiagnosticsState(), +}; +// clang-format on + [[noreturn]] void giveUp() { fprintf(stderr, "Conversion aborted after %ju error%s\n", nbErrors, nbErrors == 1 ? "" : "s"); @@ -50,3 +65,33 @@ void fatal(char const *fmt, ...) { giveUp(); } + +void warning(WarningID id, char const *fmt, ...) { + char const *flag = warnings.warningFlags[id].name; + va_list ap; + + switch (warnings.getWarningBehavior(id)) { + case WarningBehavior::DISABLED: + break; + + case WarningBehavior::ENABLED: + fprintf(stderr, "warning: [-W%s]\n ", flag); + va_start(ap, fmt); + vfprintf(stderr, fmt, ap); + va_end(ap); + putc('\n', stderr); + break; + + case WarningBehavior::ERROR: + fprintf(stderr, "error: [-Werror=%s]\n ", flag); + va_start(ap, fmt); + vfprintf(stderr, fmt, ap); + va_end(ap); + putc('\n', stderr); + + if (nbErrors != std::numeric_limits::max()) { + nbErrors++; + } + break; + } +} diff --git a/test/gfx/warn_embedded.err b/test/gfx/warn_embedded.err new file mode 100644 index 00000000..0adb25a0 --- /dev/null +++ b/test/gfx/warn_embedded.err @@ -0,0 +1,3 @@ +error: [-Werror=embedded] + Sorting palette colors by PNG's embedded PLTE chunk without '-c/--colors embedded' +Conversion aborted after 1 error diff --git a/test/gfx/warn_embedded.flags b/test/gfx/warn_embedded.flags new file mode 100644 index 00000000..1abc0b59 --- /dev/null +++ b/test/gfx/warn_embedded.flags @@ -0,0 +1 @@ +-Werror=embedded diff --git a/test/gfx/warn_embedded.png b/test/gfx/warn_embedded.png new file mode 100644 index 00000000..bb7b65d2 Binary files /dev/null and b/test/gfx/warn_embedded.png differ diff --git a/test/gfx/warn_trim_nonempty.err b/test/gfx/warn_trim_nonempty.err new file mode 100644 index 00000000..0239e12a --- /dev/null +++ b/test/gfx/warn_trim_nonempty.err @@ -0,0 +1,2 @@ +warning: [-Wtrim-nonempty] + Trimming a nonempty tile (configure with '-x/--trim-end' diff --git a/test/gfx/warn_trim_nonempty.flags b/test/gfx/warn_trim_nonempty.flags new file mode 100644 index 00000000..9366be44 --- /dev/null +++ b/test/gfx/warn_trim_nonempty.flags @@ -0,0 +1,2 @@ +-Wtrim-nonempty +-x 2 diff --git a/test/gfx/warn_trim_nonempty.out.2bpp b/test/gfx/warn_trim_nonempty.out.2bpp new file mode 100644 index 00000000..8d2d9ad3 Binary files /dev/null and b/test/gfx/warn_trim_nonempty.out.2bpp differ diff --git a/test/gfx/warn_trim_nonempty.png b/test/gfx/warn_trim_nonempty.png new file mode 100644 index 00000000..de5a5849 Binary files /dev/null and b/test/gfx/warn_trim_nonempty.png differ