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

Add QOI format support #199

Merged
merged 2 commits into from
Dec 28, 2021
Merged

Add QOI format support #199

merged 2 commits into from
Dec 28, 2021

Conversation

hovertank3d
Copy link

@sezero
Copy link
Contributor

sezero commented Dec 5, 2021

If @slouken or @icculus wants this, this patch needs changes:

  • https://github.com/phoboslab/qoi/blob/master/README.md has scary
    warnings a.out the project's readiness
  • qoi.h is not buildable in c90 mode: See the attached patch which
    makes it build in c90 mode.
  • None of the libc/libm func redefines in IMG_qoi.c are needed:
    We only need to define QOI_MALLOC and QOI_FREE.
  • We don't need qoi.h stdio functionality in IMG_qoi.c: we should
    define QOI_NO_STDIO.
  • qoi.h stores ftell result as int but it should be long,
    rest of its api (e.g. qoi_encode) relies on an int size ptr.
    That's minor, but wanted to mention anyway.
  • autotools changes are wrong: You should only touch Makefile.am
    and configure.ac, the rest I can handle regenerating here.
  • VisualC and Xcode projects need updating for the new addition.

Here are the patches I used:

@sezero
Copy link
Contributor

sezero commented Dec 5, 2021

Additional note: My gcc emits the following warnings:

qoi.h: In function ‘qoi_encode’:
qoi.h:356: warning: missing braces around initializer
qoi.h:356: warning: (near initialization for ‘index[0]’)
qoi.h: In function ‘qoi_decode’:
qoi.h:497: warning: missing braces around initializer
qoi.h:497: warning: (near initialization for ‘index[0]’)

These are qoi_rgba_t index[64] = {0}; and a compiler (e.g. MSVC)
can optimize those to memset(index,0,sizeof(x)) which can be a
problem in SDL's MSVC builds (SDL uses its own SDL_memset proc.)
I suggest that qoi project should expose a QOI_ZERO macro and
use it instead of those initializers, if it does that then SDL can
use its own SDL_zeroa macro for it.

@sezero
Copy link
Contributor

sezero commented Dec 5, 2021

FWIW, I submitted my proposed qoi.h changes to mainstream at phoboslab/qoi#67

@slouken
Copy link
Collaborator

slouken commented Dec 6, 2021

@sezero, thanks for the review. The README states 12/20/2021 as the date that the specification will be finalized, so let's wait for that before applying this patch. Hopefully the upstream patches will have landed as well.

@sezero
Copy link
Contributor

sezero commented Dec 6, 2021

And here is a patch to address zero-initializer issue I mentioned above (also submitted to upstream):
patch-zero-initializer.txt

@sezero
Copy link
Contributor

sezero commented Dec 13, 2021

The patches I suggested to qoi mainstream are in, so I updated this PR
to reflect them, along with the build system updates I requested. MSVC
and Xcode project files still need updating, by someone other than me.

@slouken: The rest is up to you.

Co-authored-by: Ozkan Sezer <[email protected]>
@sezero
Copy link
Contributor

sezero commented Dec 20, 2021

The format seems to have finalized: phoboslab/qoi#95

I updated the pull request a final time.

@slouken slouken merged commit ed7f669 into libsdl-org:main Dec 28, 2021
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

Successfully merging this pull request may close these issues.

3 participants