mobius3 / kiwi

KiWi: Killer Widgets
zlib License
186 stars 19 forks source link

Surface/Texture screw-ups #17

Closed wasamasa closed 8 years ago

wasamasa commented 8 years ago

KW_GetTilesetSurface in KW_gui.h specifies that it returns a KW_Surface*, yet it doesn't in the implementation. There's a number of other surface/texture screw-ups in the docs (see KW_GetWidgetTilesetSurface for another one). I find this pretty confusing as I've learned from experimenting with SDL that surfaces and textures are not interchangeable, in fact surfaces are the basis for creating GPU-accelerated textures.

Somewhat related question: With this in mind, what exactly are the texture-related API functions (like, KW_GetTilesetTexture or KW_GetWidgetTilesetTexture) for?

mobius3 commented 8 years ago

Thanks for catching that. It should've been KW_Surface all along. The problem is that KW_Surface and KW_Texture are both typedefs to void, thus the compiler does not complain. They are void to allow render drivers to implement them differently, but I think I'll make them complete types with a void * priv member. They are not interchangeable, indeed.

KW_Set{Widget}TilesetSurface converts the surface to a texture, making it avaliable through KW_Get{Widget}TilesetTexture (note that there is not a SetTilesetTexture family of functions, tileset textures can come only from their counterpart surfaces). It is like so to allow widget implementations to use the tileset surface to create another surface (KW_Frame is an example, it creates another surface with the rendered frame and then creates a texture of it). Other widgets might want to RenderCopy relevant parts of the tileset texture directly. Thats what those functions are for.

mobius3 commented 8 years ago

Thanks. It was long and painful, but I believe I've fixed all texture/surface/fonts mismatches because of those 'typedef void', all stuff remaining from the switch to the renderdriver port.

wasamasa commented 8 years ago

Thanks! I think I understand the surface/texture functions a bit better now. The reason why I'm asking is because I've combined several getter/setter pairs in my wrapper to generalized setters (like setf in CL or lenses in Haskell) and wanted to make sure whether it would make any sense for the example in the original post. Apparently that's not the case.

Regarding the renaming of all identifiers, the only omission I could spot is the documentation comment for KW_GetLabelFont which still speaks of a TTF_Font. I'm currently wondering how much sense it makes to include the SDL.h and SDL_ttf.h headers in the public header files, but that's probably material for a different issue.

mobius3 commented 8 years ago

I'm currently wondering how much sense it makes to include the SDL.h and SDL_ttf.h headers in the public header files

Zero. They should not be there, except for the SDL render driver. I'll go through them seeing if I can remove them.

mobius3 commented 8 years ago

I could not remove SDL.h from KW_widget.h as one of its eventhandlers (keyup/down) uses SDL_keysym. I suppose they can be wrapped/renamed, question is, where is the best place.

A KW_eventdriver seems even more pressing now, but that is a change for the future (it is as big as the renderdriver change).

wasamasa commented 8 years ago

Ah, fun, I'm currently putting up a fight with SDL_Scancode and the CHICKEN FFI.

mobius3 commented 8 years ago

Good luck, please say if I there's anything that can be done :)

wasamasa commented 8 years ago

I suspect not as the FFI handles a small set of types and cannot represent a function pointer to something taking a SDL_Scancode type. If I change it to the equivalent type, I either get complaints that enum SDL_Scancode is invalid syntax inside a function signature or that int makes it an incompatible pointer type. Therefore I've decided to not include wrappers to keydown and keyup events for the time being.