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

8bit SDL_Surfaces aren't supported when created using SDL_CreateRGBSurface #754

Closed
jorisvddonk opened this issue Dec 13, 2012 · 6 comments

Comments

Projects
None yet
2 participants
@jorisvddonk
Copy link

commented Dec 13, 2012

The SDL_CreateRGBSurface function allows for a bit depth to be set, but the JavaScript implementation of this function completely ignores this bitdepth function parameter.

#include <stdio.h>
#include <SDL/SDL.h>
#include <assert.h>

int main() {
  SDL_Init(SDL_INIT_VIDEO);
  SDL_Surface *screen = SDL_CreateRGBSurface(SDL_HWSURFACE, 320, 240, 8, 0, 0, 0, 0);
  printf("BPP is %d\n", screen->format->BitsPerPixel);
  assert(screen->format->BitsPerPixel == 8);
  SDL_Quit();

  return 0;
}

The above code returns "BPP is 8" when compiled natively, but "BPP is 32" when compiled with Emscripten.

@caiiiycuk

This comment has been minimized.

Copy link
Contributor

commented Jan 26, 2013

Hmm, i used 8bpp surface in my projects and it is works fine for me. Implementation of emscripten don`t use BitsPerPixel internally it uses SDL_HWSURFACE flag only. May be you should simple ignore this. I try to look what happen with format descriptor later...

@caiiiycuk

This comment has been minimized.

Copy link
Contributor

commented Jan 27, 2013

I made mistake with flag names, currently SLD support 8bpp surface only when SDL_HWPALETTE flag set.

#include <stdio.h>
#include <SDL/SDL.h>
#include <assert.h>

int main() {
  SDL_Init(SDL_INIT_VIDEO);
  SDL_Surface *screen = SDL_CreateRGBSurface(SDL_HWSURFACE | SDL_HWPALETTE, 320, 240, 8, 0, 0, 0, 0);
  printf("BPP is %d\n", screen->format->BitsPerPixel);
  assert(screen->format->BitsPerPixel == 8);
  SDL_Quit();

  return 0;
}

Works as expected.

@jorisvddonk

This comment has been minimized.

Copy link
Author

commented Jan 27, 2013

Thanks. This seems to indeed work (screen->format->BitsPerPixel == 8), although I'm not entirely sure if this is the normal way of dealing with 8bpp surfaces in SDL.

Unfortunately, I can't seem to manipulate the pixels array of the resulting SDL_Surface. Normally, you'd lock the surface, write to the pixels array, and then unlock the surface. However, when I try to do that I get an error at runtime: "CopyOnLock is not supported for SDL_LockSurface with SDL_HWPALETTE flag set". The relevant test (https://github.com/kripken/emscripten/blob/master/tests/sdl_canvas_palette_2.c) crashes for me, as well, with the same error message.

If I don't lock and unlock the SDL_Surface, I don't get any output at all (just a white screen), so that doesn't work either. Do you happen to know how I can manually edit the pixels array and still have my changes show up?

@caiiiycuk

This comment has been minimized.

Copy link
Contributor

commented Jan 28, 2013

Copy on lock is not supported for now. But it can be easily implemented if needed. To avoid this you should set flag copy on lock = false.

Module['preRun'] = function() {
    SDL.defaults.copyOnLock = false;
}

I think test should be fixed to work properly with copyOnLock flag. In my project i use surface in order:

//-- Initialization
createSurface()
//-- Game Loop 
draw stuff()
SDL_LockSurface(screen);
SDL_UnlockSurface(screen);
//--

SDL_LockSurface copy canvas pixels to backend storage when copyOnLock == true, in other way SDL_LockSurface do nothing.
SDL_UnlockSurface copy backend pixels to canvas.

copyOnLock only needed if you want to draw on canvas through HTML/JS and С++ code

Also copyOnLock == true gives a performance penalty

@caiiiycuk

This comment has been minimized.

Copy link
Contributor

commented Jan 28, 2013

To run test canvas_2 you also can set default == false in emscripten soruces https://github.com/kripken/emscripten/blob/master/src/library_sdl.js

var LibrarySDL = {
  $SDL__deps: ['$FS', '$Browser'],
  $SDL: {
    defaults: {
      width: 320,
      height: 200,
      copyOnLock: true // -> set to false, or in pre.js
    },

@jorisvddonk

This comment has been minimized.

Copy link
Author

commented Jan 28, 2013

Thank you very much. Setting copyOnLock to false allows me to directly edit the pixels of SDL Surfaces.

I'll close this issue, since an acceptable (to me) workaround has been found.

JoeOsborn pushed a commit to gamecip/em-fceux that referenced this issue Jan 12, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.