[Scummvm-devel] Fwd: [Scummvm-cvs-logs] SF.net SVN: scummvm: scummvm/trunk/backends/platform
aquadran at xtr.net.pl
Tue Nov 10 18:21:27 CET 2009
On 2009-11-10, at 13:27, Pawel Kolodziejski wrote:
> On 2009-11-09, at 23:15, Max Horn wrote:
>> Am 09.11.2009 um 22:35 schrieb Max Horn:
>>> Anfang der weitergeleiteten E-Mail:
>>>> Von: Pawel Kolodziejski <aquadran at xtr.net.pl>
>>>> Datum: 9. November 2009 17:21:14 MEZ
>>>> An: Max Horn <max at quendi.de>
>>>> Betreff: Re: [Scummvm-cvs-logs] SF.net SVN: scummvm:
>>>> On 2009-11-09, at 15:58, Max Horn wrote:
>>>>> Am 09.11.2009 um 15:29 schrieb aquadran at users.sourceforge.net:
>>>>>> Revision: 45775
>>>>>> Author: aquadran
>>>>>> Date: 2009-11-09 14:29:53 +0000 (Mon, 09 Nov 2009)
>>>>>> Log Message:
>>>>>> added samsung tv backend
>>>>>> Added Paths:
>>>>> This backend seems to duplicate most of the SDL backend -- very
>>>>> ugly. Any particular reason it couldn't subclass the SDL backend,
>>>>> instead of duplicating all that code?
>>>> It's in progress, so subclass not yet, and there are small changes
>>>> which does not allow much of methods as subclass. Also I don't want
>>>> put hacks into main sdl code.
>> Well, this new backend is still 95% identical to the SDL backend. I
>> find this rather aggravating :/. As we make changes to the SDL
>> backend, they will start to deviate further, though. I am scared by
>> the prospect of having this SDL backend clone in there and starting
>> bitrot, just like ther old sdl-opengl backend. The SDL backend
>> contains tons of #ifdefs etc., so on the one hand it is not nice to
>> had more, but on the other hand, it's not really making things
>> But looking at your changes specifically, it seems to me that it
>> still be far preferable and indeed quite doable to realize this as a
>> subclassed backend, by turning only a few key methods into virtual
>> ones, resp. factoring out some code that you have to implement
>> differently for your backend into new virtual methods. This will lead
>> to much better code on the long run.
>> Such a refactoring would be a nice small step towards refactoring the
>> SDL backend to make life better for backends derived from it, such as
>> the WinCE & Symbian backends.
>> Disregarding the issues I have with the actual code, there is one
>> other thing: I really really *really* dislike learning about a new
>> backend being added via the commit message. This is just as bad as if
>> it happens for an engine. This is really not good practice. In the
>> future, I expect everybody who adds major new components like
>> or engines to announce these *beforehand* on -devel, and wait for
>> approval. Getting approval from either me or Eugene alone is not
>> enough, this must go over -devel. Anyway, it's too late for that now;
>> but please at the very least, write an email to -devel now which
>> explains your new backend, what it is good for, and what your plans
>> are to address the concerns I voiced above.
> Ok, I'll start review source code about code duplication and why.
> "handleKbdMouse" use SDL_WarpMouse which I can't use, this
> shows SDL mouse cursor, only avoid it it's simply not call it.
> I tried change cursor to transparent but I still getting some
> You may wonder why not disable show mouse cursor. Unfortunately
> SDL_ShowCursor crash SDL.
> "pollEvent" looks the same for now, but later it will handle at here
> (SDL_PollEvent(&ev)) check for events from usb mouse and keyboard.
> This code will be implemented in custom way by reading Linux input
> from Linux device.
> "remapKey" not need much comments. Using remote TV controller is not
> so nice,
> but still want it as fallback due, usb mouse and keyboard support in
> TV is not enabled by default.
> It's needed load kernel modules and create device nodes in filesystem
> for use.
> And only by custom program who handle it.
> "scalersMagn" array is static in main sdl code, and later code
> blitCursor use it.
> similiar things is with "cursorStretch200To240" func
> It's temporary instad scaler without filtering, I can't change from
> gui to diffrent
> scaler, selecting mode from listview required press two buttons at
> Most probably remote controller can't send two keycode.
> has fixed list of pixel format. It's needed for emulating _hwscreen
> hmm, forgot to remove
> static and need to do fixed list of supported resolutions.
> samsung sdl has empty list defined.
> So I need my self create such list.
> Whole thing depend on emulating _hwscreen surface.
> SDL tv support practicaly only 32 bit video mode.
> So I can't use 16 bit mode.
> I can't image adding support pixel format conversion on scalers and
> other "memcpy" code.
> I think it's too much complicate, so I decided by simply adding
> compatible surface.
> In this case I changed _hwscreen to _prehwscreen. However this could
> be reverted in
> some methods where that is only thing changed, and methods would not
> need "subclassed".
> So SDL_SetVideoMode would return to _prehwscreen (propably better name
> like _videoscreen).
> And below instead _prehwscreen replace to _hwscreen.
> Maybe you would better solution, but propably that will need change
> code in main sdl.
> "unloadGFXMode" and "hotswapGFXMode"
> due freeing _prehwscreen
> due _prehwscreen instead _hwscreen, but names could be reverted as I
> mention above.
> and because emulating additional surface I used this:
> SDL_BlitSurface(_prehwscreen, 0, _hwscreen, 0);
> SDL_UpdateRect(_hwscreen, 0, 0, _hwscreen->w, _hwscreen->h);
> There is still unknown thing regarding update rects in SDL code.
> Code ignore rects and blit whole surface. I need look close into SDL
> source code
> to know if any benefits still using rects or simply dropping.
> Samsung tv support only fullscreen code.
> Regarding this It's not clear to me why gui still allow set fullscreen
> "setMouseCursor" and "blitCursor"
> due fixed pixelformat
> due usage _prehwscreen
> Whole code I don't know what to do yet. I keep normal full keyboard
> and map remote controller keys too.
> I don't understand that code yet, so I don't know if I'll change this
> yet or not.
> there is not main() here only Game_Main()
> samsung tv software call custom programs and expect it as shared
> library with
> exported entry Game_Main. Also no params for scummvm_main too.
> timer_handler, AspectRatio and other related arrays needed by
> can't call SDL_ShowCursor as I mentioned before.
> Setting scaler to different is temporary as I mentioned before.
> Now I see I need remove OSystem::initBackend(); from here too.
Of course that need stay, I miss read it.
More information about the Scummvm-devel