From: <m97...@us...> - 2013-04-16 20:10:25
|
Revision: 13238 http://sourceforge.net/p/openmsx/code/13238 Author: m9710797 Date: 2013-04-16 19:57:07 +0000 (Tue, 16 Apr 2013) Log Message: ----------- Remove findLayer() calls from Video9000::recalc() This is a followup on revision 13237, see that commit message for why using findLayer() is bad. Next to the stuff mentioned in rev13237, using findLayer() also won't work in case there are multiple V99x8 (or V9990) chips in one MSX machine (we'd like to support this in the near(?) future). This patch adds a device reference to the V99x8 in the video9000 config file (it already had a reference to the V9990). So even if there are multiple V99x8/V9990 chips in the same machine, the config now refers to a specific chip. So once the Video9000 device knows about the VDP/V9990 device, it can query that device for the specific Layer (=PostProcessor) it is rendering to. And thus this allows to get rid of the findLayer() calls with hardcoded layer names. Although this change is conceptually simple, this patch is quite large because: - The new getPostProcessor() call has to pass through 2 indirection layers (renderer and rasterizer). - Each indirection has an abstract base class, sometimes with a dummy implementation. - This change is required on both the VDP and V9990 side. - All these changes touch both the .hh and .cc file Revision Links: -------------- http://sourceforge.net/p/openmsx/code/13237 http://sourceforge.net/p/openmsx/code/13237 Modified Paths: -------------- openmsx/trunk/share/extensions/video9000/hardwareconfig.xml openmsx/trunk/src/video/DummyRenderer.cc openmsx/trunk/src/video/DummyRenderer.hh openmsx/trunk/src/video/PixelRenderer.cc openmsx/trunk/src/video/PixelRenderer.hh openmsx/trunk/src/video/Rasterizer.hh openmsx/trunk/src/video/Renderer.hh openmsx/trunk/src/video/SDLRasterizer.cc openmsx/trunk/src/video/SDLRasterizer.hh openmsx/trunk/src/video/VDP.cc openmsx/trunk/src/video/VDP.hh openmsx/trunk/src/video/v9990/V9990.cc openmsx/trunk/src/video/v9990/V9990.hh openmsx/trunk/src/video/v9990/V9990DummyRenderer.cc openmsx/trunk/src/video/v9990/V9990DummyRenderer.hh openmsx/trunk/src/video/v9990/V9990PixelRenderer.cc openmsx/trunk/src/video/v9990/V9990PixelRenderer.hh openmsx/trunk/src/video/v9990/V9990Rasterizer.hh openmsx/trunk/src/video/v9990/V9990Renderer.hh openmsx/trunk/src/video/v9990/V9990SDLRasterizer.cc openmsx/trunk/src/video/v9990/V9990SDLRasterizer.hh openmsx/trunk/src/video/v9990/Video9000.cc openmsx/trunk/src/video/v9990/Video9000.hh Modified: openmsx/trunk/share/extensions/video9000/hardwareconfig.xml =================================================================== --- openmsx/trunk/share/extensions/video9000/hardwareconfig.xml 2013-04-15 19:33:17 UTC (rev 13237) +++ openmsx/trunk/share/extensions/video9000/hardwareconfig.xml 2013-04-16 19:57:07 UTC (rev 13238) @@ -16,6 +16,7 @@ </V9990> <Video9000 id="Video9000"> <io base="0x6f" num="0x01"/> + <device idref="VDP"/> <device idref="Sunrise GFX9000"/> </Video9000> </devices> Modified: openmsx/trunk/src/video/DummyRenderer.cc =================================================================== --- openmsx/trunk/src/video/DummyRenderer.cc 2013-04-15 19:33:17 UTC (rev 13237) +++ openmsx/trunk/src/video/DummyRenderer.cc 2013-04-16 19:57:07 UTC (rev 13238) @@ -5,6 +5,10 @@ namespace openmsx { +PostProcessor* DummyRenderer::getPostProcessor() const { + return nullptr; +} + void DummyRenderer::reInit() { } Modified: openmsx/trunk/src/video/DummyRenderer.hh =================================================================== --- openmsx/trunk/src/video/DummyRenderer.hh 2013-04-15 19:33:17 UTC (rev 13237) +++ openmsx/trunk/src/video/DummyRenderer.hh 2013-04-16 19:57:07 UTC (rev 13238) @@ -14,6 +14,7 @@ { public: // Renderer interface: + PostProcessor* getPostProcessor() const; void reInit(); void frameStart(EmuTime::param time); void frameEnd(EmuTime::param time); Modified: openmsx/trunk/src/video/PixelRenderer.cc =================================================================== --- openmsx/trunk/src/video/PixelRenderer.cc 2013-04-15 19:33:17 UTC (rev 13237) +++ openmsx/trunk/src/video/PixelRenderer.cc 2013-04-16 19:57:07 UTC (rev 13238) @@ -137,6 +137,11 @@ renderSettings.getMaxFrameSkip().detach(*this); } +PostProcessor* PixelRenderer::getPostProcessor() const +{ + return rasterizer->getPostProcessor(); +} + void PixelRenderer::reInit() { // Don't draw before frameStart() is called. Modified: openmsx/trunk/src/video/PixelRenderer.hh =================================================================== --- openmsx/trunk/src/video/PixelRenderer.hh 2013-04-15 19:33:17 UTC (rev 13237) +++ openmsx/trunk/src/video/PixelRenderer.hh 2013-04-16 19:57:07 UTC (rev 13238) @@ -33,6 +33,7 @@ virtual ~PixelRenderer(); // Renderer interface: + virtual PostProcessor* getPostProcessor() const; virtual void reInit(); virtual void frameStart(EmuTime::param time); virtual void frameEnd(EmuTime::param time); Modified: openmsx/trunk/src/video/Rasterizer.hh =================================================================== --- openmsx/trunk/src/video/Rasterizer.hh 2013-04-15 19:33:17 UTC (rev 13237) +++ openmsx/trunk/src/video/Rasterizer.hh 2013-04-16 19:57:07 UTC (rev 13238) @@ -9,6 +9,7 @@ namespace openmsx { +class PostProcessor; class RawFrame; class Rasterizer @@ -16,6 +17,9 @@ public: virtual ~Rasterizer() {} + /** See VDP::getPostProcessor(). */ + virtual PostProcessor* getPostProcessor() const = 0; + /** Will the output of this Rasterizer be displayed? * There is no point in producing a frame that will not be displayed. * TODO: Is querying the next pipeline step the best way to solve this, Modified: openmsx/trunk/src/video/Renderer.hh =================================================================== --- openmsx/trunk/src/video/Renderer.hh 2013-04-15 19:33:17 UTC (rev 13237) +++ openmsx/trunk/src/video/Renderer.hh 2013-04-16 19:57:07 UTC (rev 13238) @@ -8,6 +8,7 @@ namespace openmsx { +class PostProcessor; class DisplayMode; class RawFrame; @@ -24,6 +25,9 @@ public: virtual ~Renderer(); + /** See VDP::getPostProcessor. */ + virtual PostProcessor* getPostProcessor() const = 0; + /** Reinitialise Renderer state. */ virtual void reInit() = 0; Modified: openmsx/trunk/src/video/SDLRasterizer.cc =================================================================== --- openmsx/trunk/src/video/SDLRasterizer.cc 2013-04-15 19:33:17 UTC (rev 13237) +++ openmsx/trunk/src/video/SDLRasterizer.cc 2013-04-16 19:57:07 UTC (rev 13238) @@ -112,6 +112,12 @@ } template <class Pixel> +PostProcessor* SDLRasterizer<Pixel>::getPostProcessor() const +{ + return postProcessor.get(); +} + +template <class Pixel> bool SDLRasterizer<Pixel>::isActive() { return postProcessor->needRender() && Modified: openmsx/trunk/src/video/SDLRasterizer.hh =================================================================== --- openmsx/trunk/src/video/SDLRasterizer.hh 2013-04-15 19:33:17 UTC (rev 13237) +++ openmsx/trunk/src/video/SDLRasterizer.hh 2013-04-16 19:57:07 UTC (rev 13238) @@ -38,6 +38,7 @@ virtual ~SDLRasterizer(); // Rasterizer interface: + virtual PostProcessor* getPostProcessor() const; virtual bool isActive(); virtual void reset(); virtual void frameStart(EmuTime::param time); Modified: openmsx/trunk/src/video/VDP.cc =================================================================== --- openmsx/trunk/src/video/VDP.cc 2013-04-15 19:33:17 UTC (rev 13237) +++ openmsx/trunk/src/video/VDP.cc 2013-04-16 19:57:07 UTC (rev 13238) @@ -333,6 +333,11 @@ vram->setRenderer(renderer.get(), frameStartTime.getTime()); } +PostProcessor* VDP::getPostProcessor() const +{ + return renderer->getPostProcessor(); +} + void VDP::resetInit() { // note: vram, spriteChecker, cmdEngine, renderer may not yet be Modified: openmsx/trunk/src/video/VDP.hh =================================================================== --- openmsx/trunk/src/video/VDP.hh 2013-04-15 19:33:17 UTC (rev 13237) +++ openmsx/trunk/src/video/VDP.hh 2013-04-16 19:57:07 UTC (rev 13238) @@ -15,6 +15,7 @@ namespace openmsx { +class PostProcessor; class Renderer; class VDPCmdEngine; class VDPVRAM; @@ -82,6 +83,12 @@ virtual void writeIO(word port, byte value, EmuTime::param time); virtual void executeUntil(EmuTime::param time, int userData); + /** Used by Video9000 to be able to couple the VDP and V9990 output. + * Can return nullptr in case of renderer=none. This value can change + * over the lifetime of the VDP object (on renderer switch). + */ + PostProcessor* getPostProcessor() const; + /** Is this an MSX1 VDP? * @return True if this is an MSX1 VDP (TMS99X8A or TMS9929A), * False otherwise. Modified: openmsx/trunk/src/video/v9990/V9990.cc =================================================================== --- openmsx/trunk/src/video/v9990/V9990.cc 2013-04-15 19:33:17 UTC (rev 13237) +++ openmsx/trunk/src/video/v9990/V9990.cc 2013-04-16 19:57:07 UTC (rev 13238) @@ -128,6 +128,11 @@ display.detach(*this); } +PostProcessor* V9990::getPostProcessor() const +{ + return renderer->getPostProcessor(); +} + // ------------------------------------------------------------------------- // MSXDevice // ------------------------------------------------------------------------- Modified: openmsx/trunk/src/video/v9990/V9990.hh =================================================================== --- openmsx/trunk/src/video/v9990/V9990.hh 2013-04-15 19:33:17 UTC (rev 13237) +++ openmsx/trunk/src/video/v9990/V9990.hh 2013-04-16 19:57:07 UTC (rev 13238) @@ -17,6 +17,7 @@ namespace openmsx { +class PostProcessor; class Display; class V9990VRAM; class V9990CmdEngine; @@ -42,6 +43,12 @@ virtual byte peekIO(word port, EmuTime::param time) const; virtual void writeIO(word port, byte value, EmuTime::param time); + /** Used by Video9000 to be able to couple the VDP and V9990 output. + * Can return nullptr in case of renderer=none. This value can change + * over the lifetime of the V9990 object (on renderer switch). + */ + PostProcessor* getPostProcessor() const; + /** Obtain a reference to the V9990's VRAM */ inline V9990VRAM& getVRAM() { Modified: openmsx/trunk/src/video/v9990/V9990DummyRenderer.cc =================================================================== --- openmsx/trunk/src/video/v9990/V9990DummyRenderer.cc 2013-04-15 19:33:17 UTC (rev 13237) +++ openmsx/trunk/src/video/v9990/V9990DummyRenderer.cc 2013-04-16 19:57:07 UTC (rev 13238) @@ -4,6 +4,11 @@ namespace openmsx { +PostProcessor* V9990DummyRenderer::getPostProcessor() const +{ + return nullptr; +} + void V9990DummyRenderer::reset(EmuTime::param /*time*/) { } Modified: openmsx/trunk/src/video/v9990/V9990DummyRenderer.hh =================================================================== --- openmsx/trunk/src/video/v9990/V9990DummyRenderer.hh 2013-04-15 19:33:17 UTC (rev 13237) +++ openmsx/trunk/src/video/v9990/V9990DummyRenderer.hh 2013-04-16 19:57:07 UTC (rev 13238) @@ -11,6 +11,7 @@ { public: // V9990Renderer interface: + PostProcessor* getPostProcessor() const; void reset(EmuTime::param time); void frameStart(EmuTime::param time); void frameEnd(EmuTime::param time); Modified: openmsx/trunk/src/video/v9990/V9990PixelRenderer.cc =================================================================== --- openmsx/trunk/src/video/v9990/V9990PixelRenderer.cc 2013-04-15 19:33:17 UTC (rev 13237) +++ openmsx/trunk/src/video/v9990/V9990PixelRenderer.cc 2013-04-16 19:57:07 UTC (rev 13238) @@ -48,6 +48,11 @@ renderSettings.getMinFrameSkip().detach(*this); } +PostProcessor* V9990PixelRenderer::getPostProcessor() const +{ + return rasterizer->getPostProcessor(); +} + void V9990PixelRenderer::reset(EmuTime::param time) { displayEnabled = vdp.isDisplayEnabled(); Modified: openmsx/trunk/src/video/v9990/V9990PixelRenderer.hh =================================================================== --- openmsx/trunk/src/video/v9990/V9990PixelRenderer.hh 2013-04-15 19:33:17 UTC (rev 13237) +++ openmsx/trunk/src/video/v9990/V9990PixelRenderer.hh 2013-04-16 19:57:07 UTC (rev 13238) @@ -32,6 +32,7 @@ virtual ~V9990PixelRenderer(); // V9990Renderer interface: + PostProcessor* getPostProcessor() const; void reset(EmuTime::param time); void frameStart(EmuTime::param time); void frameEnd(EmuTime::param time); Modified: openmsx/trunk/src/video/v9990/V9990Rasterizer.hh =================================================================== --- openmsx/trunk/src/video/v9990/V9990Rasterizer.hh 2013-04-15 19:33:17 UTC (rev 13237) +++ openmsx/trunk/src/video/v9990/V9990Rasterizer.hh 2013-04-16 19:57:07 UTC (rev 13238) @@ -9,6 +9,8 @@ namespace openmsx { +class PostProcessor; + /** If this seems awfully familiar, take a look at Rasterizer.hh * It's virtually the same class, but for a different video processor. */ @@ -17,6 +19,9 @@ public: virtual ~V9990Rasterizer() {} + /** See V9990::getPostProcessor(). */ + virtual PostProcessor* getPostProcessor() const = 0; + /** Will the output of this Rasterizer be displayed? * There is no point in producing a frame that will not be displayed. * TODO: Is querying the next pipeline step the best way to solve this, Modified: openmsx/trunk/src/video/v9990/V9990Renderer.hh =================================================================== --- openmsx/trunk/src/video/v9990/V9990Renderer.hh 2013-04-15 19:33:17 UTC (rev 13237) +++ openmsx/trunk/src/video/v9990/V9990Renderer.hh 2013-04-16 19:57:07 UTC (rev 13238) @@ -9,6 +9,8 @@ namespace openmsx { +class PostProcessor; + /** Abstract base class for V9990 renderers. * A V9990Renderer is a class that covnerts the V9990 state into * visual information (e.g. pixels on a screen). @@ -20,6 +22,9 @@ public: virtual ~V9990Renderer(); + /** See V9990::getPostProcessor. */ + virtual PostProcessor* getPostProcessor() const = 0; + /** Re-initialise the V9990Renderer's state. * @param time The moment in emulated time this reset occurs. */ Modified: openmsx/trunk/src/video/v9990/V9990SDLRasterizer.cc =================================================================== --- openmsx/trunk/src/video/v9990/V9990SDLRasterizer.cc 2013-04-15 19:33:17 UTC (rev 13237) +++ openmsx/trunk/src/video/v9990/V9990SDLRasterizer.cc 2013-04-16 19:57:07 UTC (rev 13238) @@ -60,6 +60,12 @@ } template <class Pixel> +PostProcessor* V9990SDLRasterizer<Pixel>::getPostProcessor() const +{ + return postProcessor.get(); +} + +template <class Pixel> bool V9990SDLRasterizer<Pixel>::isActive() { return postProcessor->needRender() && Modified: openmsx/trunk/src/video/v9990/V9990SDLRasterizer.hh =================================================================== --- openmsx/trunk/src/video/v9990/V9990SDLRasterizer.hh 2013-04-15 19:33:17 UTC (rev 13237) +++ openmsx/trunk/src/video/v9990/V9990SDLRasterizer.hh 2013-04-16 19:57:07 UTC (rev 13238) @@ -36,6 +36,7 @@ virtual ~V9990SDLRasterizer(); // Rasterizer interface: + virtual PostProcessor* getPostProcessor() const; virtual bool isActive(); virtual void reset(); virtual void frameStart(); Modified: openmsx/trunk/src/video/v9990/Video9000.cc =================================================================== --- openmsx/trunk/src/video/v9990/Video9000.cc 2013-04-15 19:33:17 UTC (rev 13237) +++ openmsx/trunk/src/video/v9990/Video9000.cc 2013-04-16 19:57:07 UTC (rev 13238) @@ -1,6 +1,7 @@ // $Id$ #include "Video9000.hh" +#include "VDP.hh" #include "V9990.hh" #include "Reactor.hh" #include "Display.hh" @@ -18,12 +19,11 @@ Video9000::Video9000(const DeviceConfig& config) : MSXDevice(config) , VideoLayer(getMotherBoard(), VIDEO_9000) - , display(getReactor().getDisplay()) , videoSourceSetting(getMotherBoard().getVideoSource()) { EventDistributor& distributor = getReactor().getEventDistributor(); distributor.registerEventListener(OPENMSX_FINISH_FRAME_EVENT, *this); - display.attach(*this); + getReactor().getDisplay().attach(*this); activeLayer = nullptr; // we can't set activeLayer yet value = 0x10; @@ -32,21 +32,26 @@ void Video9000::init() { MSXDevice::init(); - const MSXDevice::Devices& references = getReferences(); - v9990 = references.empty() - ? nullptr - : dynamic_cast<V9990*>(references[0]); - if (!v9990) { - throw MSXException("Invalid Video9000 configuration: " - "need reference to V9990 device."); + auto references = getReferences(); // make copy + bool error = false; + if (references.size() != 2) error = true; + if (!error && !dynamic_cast<VDP*>(references[0])) { + std::swap(references[0], references[1]); // try reverse order } + if (!error) vdp = dynamic_cast<VDP* >(references[0]); + if (!error) v9990 = dynamic_cast<V9990*>(references[1]); + if (error || !vdp || !v9990) { + throw MSXException( + "Invalid Video9000 configuration: " + "need reference to VDP and V9990 device."); + } } Video9000::~Video9000() { EventDistributor& distributor = getReactor().getEventDistributor(); distributor.unregisterEventListener(OPENMSX_FINISH_FRAME_EVENT, *this); - display.detach(*this); + getReactor().getDisplay().detach(*this); } void Video9000::reset(EmuTime::param time) @@ -63,9 +68,9 @@ void Video9000::recalc() { - // TODO can we do better than name based lookup? - v99x8Layer = dynamic_cast<PostProcessor*>(display.findLayer("V99x8")); - v9990Layer = dynamic_cast<PostProcessor*>(display.findLayer("V9990")); + v99x8Layer = vdp ->getPostProcessor(); + v9990Layer = v9990->getPostProcessor(); + assert(!!v99x8Layer == !!v9990Layer); // either both or neither // ...0.... -> only V9990 // ...10... -> only V99x8 @@ -80,11 +85,7 @@ if (v9990Layer) v9990Layer->setVideo9000Active( showV9990 ? VideoLayer::ACTIVE_FRONT : VideoLayer::INACTIVE); activeLayer = showV9990 ? v9990Layer : v99x8Layer; - if (!activeLayer) { - // only happens on a MSX system with Video9000 but with - // missing V99x8 or V9990 - activeLayer = display.findLayer("snow"); - } + // activeLayer==nullptr is possible for renderer=none recalcVideoSource(); } @@ -114,6 +115,8 @@ if (!activeLayer) { recalc(); } + // activeLayer==nullptr is possible for renderer=none, but in that case + // the paint() method will never be called. activeLayer->paint(output); } @@ -124,7 +127,7 @@ void Video9000::takeRawScreenShot(unsigned height, const std::string& filename) { - auto layer = dynamic_cast<VideoLayer*>(activeLayer); + auto* layer = dynamic_cast<VideoLayer*>(activeLayer); if (!layer) { throw CommandException("TODO"); } Modified: openmsx/trunk/src/video/v9990/Video9000.hh =================================================================== --- openmsx/trunk/src/video/v9990/Video9000.hh 2013-04-15 19:33:17 UTC (rev 13237) +++ openmsx/trunk/src/video/v9990/Video9000.hh 2013-04-16 19:57:07 UTC (rev 13238) @@ -10,7 +10,7 @@ namespace openmsx { -class Display; +class VDP; class V9990; class PostProcessor; @@ -50,8 +50,8 @@ // Observer<Setting> void update(const Setting& setting); - Display& display; VideoSourceSetting& videoSourceSetting; + VDP* vdp; V9990* v9990; Layer* activeLayer; PostProcessor* v99x8Layer; This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site. |