Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Export: proposal for image format options #882

Closed
scurest opened this issue Jan 20, 2020 · 6 comments
Closed

Export: proposal for image format options #882

scurest opened this issue Jan 20, 2020 · 6 comments

Comments

@scurest
Copy link
Contributor

scurest commented Jan 20, 2020

I want to propose changes to the behaviour of the image format options in the exporter

Automatic (the default)

Basically I want to propose doing this

if export image corresponds to an existing image:
    if existing_image.file_format == 'JPEG':
        file_format = 'JPEG'
    else:
        file_format = 'PNG'
else:
    file_format = 'PNG'

IOW, this option would always favor PNGs except that (quoting from #203)

if the exporter already has access to a source PNG or JPG, it should be just copying the already-compressed data from there directly

This fixes two things:

  • Currently this option does not make good use of an already-compressed JPEG. It will only encode an existing image as a JPEG if its name ends with .jpg/.jpeg. It would be better not to use the name, but to look at the image.file_format field.

    This can make a big difference in performance. For Sponza for instance

    • exporting with Automatic, all the JPEGs are re-encoded as PNGs
      • export time: 32s
      • resulting filesize: 116M
    • exporting with JPEG Format (.jpg), all JPEGs just copied over
      • export time: 22s
      • resulting filesize: 45M
  • When an image needs to be packed from channels, the packed image can sometimes be a JPEG if one of the constituents is (note: only sometimes!). This is lossy and causes channel-bleed (AO texture influences roughtness #818). I think a better default is to prefer being lossless.

PNG format (.png)

Remove it. Since Automatic would favor PNGs, it's main difference would be that it re-encodes JPEGs as PNG. It would basically become a "make my file bigger" option.

JPEG format (.jpg)

This is okay, no change.

@emackey
Copy link
Member

emackey commented Jan 21, 2020

I like this idea. In particular I think there should never be a case where an existing JPG needlessly becomes a PNG without any channel reassignments.

@donmccurdy
Copy link
Contributor

Agreed. 👍

@scurest
Copy link
Contributor Author

scurest commented Jan 21, 2020

Okay, cool. I put up a PR for it.

emackey added a commit that referenced this issue Jan 22, 2020
Export: implement image options change (#882)
@emackey
Copy link
Member

emackey commented Jan 22, 2020

Fixed in #883.

@globglob3D
Copy link

globglob3D commented Jan 27, 2021

When an image needs to be packed from channels, the packed image can sometimes be a JPEG if one of the constituents is (note: only sometimes!). This is lossy and causes channel-bleed (#818). I think a better default is to prefer being lossless.

If all constituents are JPEG I think the resulting packed image should be JPEG too. Yes this would cause some amount of bleed but
you decrease the size of the texture by a lot (up to 10 times?). If you choose to have AO/R/N as JPEG initially it means you care more about final glb size than quality. Or perhaps we should add an option regarding packed image format in the exporter?

#1318

@donmccurdy
Copy link
Contributor

More recent discussion in: #1099

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants