Skip to content

Commit

Permalink
urcheon/action: convert png to lossless webp to workaround Pillow bug
Browse files Browse the repository at this point in the history
See #61

When loading 8-bit grayscale PNGs with Pillow, and converting them to RGB,
they are converted like if they were 1-bit PNGs.

So basically, white,lightgrey,midgrey,darkgrey,black
becomes white,white,white,white,black.

For now I'm implementing a workaround by converting all PNGs
to lossless WebP as a transient format because cwebp is known
to properly convert PNG to lossless WebP before loading the
bitmap with Pillow.

For example I do PNG to WebP to TGA to convert from PNG to TGA.

This becomes crazy because we already did PNG to TGA to CRN to convert
from PNG to CRN because crunch doesn't read all PNG formats. So, since
we cannot do PNG to TGA in one go, I now do PNG to WebP to TGA to CRN
to convert from PNG to CRN. This is really crazy!

Some code may now be highly unoptimized like PNG to WebP since the
sanitization of the image is converted first to WebP before being
converted to WebP again.

I plan to introduce a specific ImageConverter module to handle
image conversion properly with the “shortest” (ahem) path
for known formats.

This workaround is good enough to build Unvanquished 0.54 release.
  • Loading branch information
illwieckz committed Jan 29, 2023
1 parent ce0ba34 commit 07b1324
Showing 1 changed file with 62 additions and 29 deletions.
91 changes: 62 additions & 29 deletions Urcheon/Action.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,6 @@ def readActions(self, action_list=None):
Ui.print(file_path + ": Known rule, will not " + self.inspector.action_description_dict[action_name] + " (missing file).")
self.computed_disabled_action_dict[action_name].append(file_path)


def computeActions(self, file_list):
for file_path in file_list:
file_path = os.path.normpath(file_path)
Expand Down Expand Up @@ -445,6 +444,48 @@ def getStatReference(self):

return FileSystem.getNewer(file_reference_list)

# Produce an image that is properly writable.
def openAndSanitizeImage(self):

# HACK: Pillow has a bug and converts 8-bit greyscale PNG to 1-bit black and white image when converting to RGB,
# We workaround the issue by converting PNG images to lossless WebP first, to get an RGB WebP
# that will be properly loaded and converted to other RGB formats by Pillow.

source_path = self.getSourcePath()

if source_path[-4:].lower() == ".png":
build_path = self.getTargetPath()

transient_handle, transient_path = tempfile.mkstemp(suffix="_" + os.path.basename(build_path) + "_transient" + os.path.extsep + "webp")
os.close(transient_handle)

self.callProcess(["cwebp", "-v", "-mt", "-lossless", "-z", "0", source_path, "-o", transient_path])

image = Image.open(transient_path)

os.remove(transient_path)
else:
image = Image.open(source_path)

image = image.convert("RGBA")

has_alpha = False

# Strip the alpha channel if it's fully opaque.
for x in range(0, image.width):
for y in range(0, image.height):
if image.getpixel((x, y))[3] != 255:
has_alpha = True
break

if has_alpha:
break

if not has_alpha:
image = image.convert("RGB")

return image


# TODO: Catch when it is not supported and print a warning.
# It is only supported on DPK VFS.
Expand Down Expand Up @@ -524,9 +565,11 @@ def effective_run(self):
shutil.copyfile(source_path, build_path)
else:
Ui.laconic("Convert to " + self.printable_target_format + ": " + self.file_path)
image = Image.open(source_path)
image = self.openAndSanitizeImage()

# OSError: cannot write mode RGBA as JPEG
image = image.convert("RGB")

image.save(build_path, quality=self.convert_jpg_quality)

self.setTimeStamp()
Expand Down Expand Up @@ -559,7 +602,7 @@ def effective_run(self):
shutil.copyfile(source_path, build_path)
else:
Ui.laconic("Convert to png: " + self.file_path)
image = Image.open(source_path)
image = self.openAndSanitizeImage()
image.save(build_path)

self.setTimeStamp()
Expand Down Expand Up @@ -589,15 +632,19 @@ def effective_run(self):
shutil.copyfile(source_path, build_path)
else:
Ui.laconic("Convert to " + self.printable_target_format + ": " + self.file_path)

image = self.openAndSanitizeImage()

# cwebp doesn't support many input format, PNG is known to be well supported,
# so we convert the image to PNG first.
transient_handle, transient_path = tempfile.mkstemp(suffix="_" + os.path.basename(build_path) + "_transient" + os.path.extsep + "png")
os.close(transient_handle)

image = Image.open(source_path)
image.save(transient_path)

self.callProcess(["cwebp", "-v", "-mt"] + self.cwebp_extra_args + [transient_path, "-o", build_path])
if os.path.isfile(transient_path):
os.remove(transient_path)

os.remove(transient_path)

self.setTimeStamp()

Expand Down Expand Up @@ -637,42 +684,28 @@ def effective_run(self):
else:
Ui.laconic("Convert to " + self.printable_target_format + ": " + self.file_path)

# The crunch tool only supports a small number of formats, and is known to fail on some variants of the format it handles (example: PNG)
# HACK: The crunch tool only supports a small number of formats, and is
# known to fail on some variants of the format it handles (example: PNG).
# See https://github.com/DaemonEngine/crunch/issues/13
# So, better convert to TGA first.

# The TGA format produced by the ImageMagick "convert" tool is known to be broken so we don't use the "convert" tool anymore.
# The TGA format produced by the ImageMagick "convert" tool is known to
# be broken so we don't use the "convert" tool anymore.
# See https://gitlab.com/illwieckz/investigating-tga-orientation-in-imagemagick

# The image is converted to RGB or RGBA to make sure the produced TGA is readable by crunch, and does not use an unsupported TGA variant.
# The image is converted to RGB or RGBA to make sure the produced TGA
# is readable by crunch, and does not use an unsupported TGA variant.

image = self.openAndSanitizeImage()

transient_handle, transient_path = tempfile.mkstemp(suffix="_" + os.path.basename(build_path) + "_transient" + os.path.extsep + "tga")
os.close(transient_handle)

image = Image.open(source_path)
image = image.convert("RGBA")

has_alpha = False

# Strip the alpha channel if it's fully opaque.
for x in range(0, image.width):
for y in range(0, image.height):
if image.getpixel((x, y))[3] != 255:
has_alpha = True
break

if has_alpha:
break

if not has_alpha:
image = image.convert("RGB")

image.save(transient_path)

self.callProcess(["crunch", "-helperThreads", str(self.thread_count), "-noNormalDetection", "-file", transient_path] + self.crunch_extra_args + ["-quality", "255", "-out", build_path])

if os.path.isfile(transient_path):
os.remove(transient_path)
os.remove(transient_path)

self.setTimeStamp()

Expand Down

0 comments on commit 07b1324

Please sign in to comment.