[ scummvm-Patches-2689887 ] SCI: Started objectifying the graphics resource manager

[SourceForge.net]

Patches item #2689887, was opened at 2009-03-17 12:27
Message generated for change (Tracker Item Submitted) made by thebluegr
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=418822&aid=2689887&group_id=37116

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: None
Group: None
Status: Open
Resolution: None
Priority: 5
Private: No
Submitted By: Filippos Karapetis (thebluegr)
Assigned to: Nobody/Anonymous (nobody)
Summary: SCI: Started objectifying the graphics resource manager

Initial Comment:
Hello

This is a fairly big change to SCI, so I wanted to get some opinions first.

Changes:
- I've started moving code outside of gfx/resource/res_manager.cpp into gfx/resmgr.cpp. In FreeSCI, it was possible to have several different graphics resource managers, but we only have one, so the code in resource/res_manager.cpp can be merged in gfx/resmgr.cpp (that's the only place it's used in) - I got feedback on this by Lars Skovlund.
- It seems that the graphics driver parameter is not needed in gfx_free_pixmap(). That method checks if the screen palette is set, and frees the associated pixmap parameter. I really don't understand why this is necessary, and I only found one place where pixmaps are freed and have invalid palettes: inside gfxr_draw_pic01(), when gfx_free_mode() frees the graphics mode, it frees the view's associated palette too. Setting the view's palette to NULL in this place after freeing it seems to be enough. By removing this parameter from gfx_free_pixmap(), several associated free() methods can be simplified (since the graphics driver parameter is removed from all of them - it was only piggybanked for gfx_free_pixmap(). 
- I've started objectifying the methods of the graphics resource manager. They all require parts of the graphics resource state, so it made sense to create a class and pass the state to the constructor (plus, all of these methods are parts of the graphics resource manager). I didn't convert all of them yet, because of the previous issue - removing the driver parameter means that more methods can be placed inside the graphics resource manager, which won't be needing the graphics driver parameter.

Are my assumptions correct? Does this kind of objectification make sense?

Comments are welcome

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=418822&aid=2689887&group_id=37116