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
Closed

Comments

@jorisvddonk
Copy link

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Author

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Author

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
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants