diff options
author | Colin Snover | 2016-03-18 13:08:37 -0500 |
---|---|---|
committer | Colin Snover | 2016-03-18 13:08:37 -0500 |
commit | e25d928a9cb82d458c7f592ec81576f0d57d8eca (patch) | |
tree | 6fc1c40adc8bebf6174fcc49f3514d576ae0a1b6 /engines/sci/graphics | |
parent | 856f3ae6489b3f9aec62043c58c3961baf619f16 (diff) | |
download | scummvm-rg350-e25d928a9cb82d458c7f592ec81576f0d57d8eca.tar.gz scummvm-rg350-e25d928a9cb82d458c7f592ec81576f0d57d8eca.tar.bz2 scummvm-rg350-e25d928a9cb82d458c7f592ec81576f0d57d8eca.zip |
SCI32: Fix crashes in kIsOnMe caused by stale CelObjs
Diffstat (limited to 'engines/sci/graphics')
-rw-r--r-- | engines/sci/graphics/celobj32.h | 4 | ||||
-rw-r--r-- | engines/sci/graphics/frameout.cpp | 9 | ||||
-rw-r--r-- | engines/sci/graphics/screen_item32.cpp | 12 |
3 files changed, 23 insertions, 2 deletions
diff --git a/engines/sci/graphics/celobj32.h b/engines/sci/graphics/celobj32.h index 600ae82d32..0bb4b03ae2 100644 --- a/engines/sci/graphics/celobj32.h +++ b/engines/sci/graphics/celobj32.h @@ -104,6 +104,10 @@ struct CelInfo32 { bitmap == other.bitmap ); } + + inline bool operator!=(const CelInfo32 &other) { + return !(*this == other); + } }; class CelObj; diff --git a/engines/sci/graphics/frameout.cpp b/engines/sci/graphics/frameout.cpp index b654cbf2b9..062cd8b600 100644 --- a/engines/sci/graphics/frameout.cpp +++ b/engines/sci/graphics/frameout.cpp @@ -1381,7 +1381,7 @@ void GfxFrameout::kernelFrameOut(const bool shouldShowBits) { #pragma mark Mouse cursor reg_t GfxFrameout::kernelIsOnMe(const reg_t object, const Common::Point &position, bool checkPixel) const { - reg_t planeObject = readSelector(_segMan, object, SELECTOR(plane)); + const reg_t planeObject = readSelector(_segMan, object, SELECTOR(plane)); Plane *plane = _visiblePlanes.findByObject(planeObject); if (plane == nullptr) { return make_reg(0, 0); @@ -1392,6 +1392,13 @@ reg_t GfxFrameout::kernelIsOnMe(const reg_t object, const Common::Point &positio return make_reg(0, 0); } + // NOTE: The original engine passed a copy of the ScreenItem into isOnMe + // as a hack around the fact that the screen items in `_visiblePlanes` + // did not have their `_celObj` pointers cleared when their CelInfo was + // updated by `Plane::decrementScreenItemArrayCounts`. We handle this + // this more intelligently by clearing `_celObj` in the copy assignment + // operator, which is only ever called by `decrementScreenItemArrayCounts` + // anyway. return make_reg(0, isOnMe(*screenItem, *plane, position, checkPixel)); } diff --git a/engines/sci/graphics/screen_item32.cpp b/engines/sci/graphics/screen_item32.cpp index c44d3e96f1..c3fdbb6845 100644 --- a/engines/sci/graphics/screen_item32.cpp +++ b/engines/sci/graphics/screen_item32.cpp @@ -115,7 +115,17 @@ _screenRect(other._screenRect) { } void ScreenItem::operator=(const ScreenItem &other) { - _celInfo = other._celInfo; + // NOTE: The original engine did not check for differences in `_celInfo` + // to clear `_celObj` here; instead, it unconditionally set `_celInfo`, + // didn't clear `_celObj`, and did hacky stuff in `kIsOnMe` to avoid + // testing a mismatched `_celObj`. See `GfxFrameout::kernelIsOnMe` for + // more detail. + if (_celInfo != other._celInfo) { + _celInfo = other._celInfo; + delete _celObj; + _celObj = nullptr; + } + _screenRect = other._screenRect; _mirrorX = other._mirrorX; _useInsetRect = other._useInsetRect; |