Skip to content

Commit

Permalink
SCI32: Clean up GfxTransitions32
Browse files Browse the repository at this point in the history 
* Use containers where appropriate
* Re-wrap doxygen comments to 80 columns
* Clarify comments for parts of the engine that are understood now
  but were not understood at the time of the initial
  implementation
  • Loading branch information
@csnover
csnover committed Oct 7, 2017
1 parent 0ac5d84 commit ff3503a
Show file tree
Hide file tree
Showing 3 changed files with 125 additions and 177 deletions.
17 changes: 8 additions & 9 deletions engines/sci/engine/kgraphics32.cpp
View file
Expand Up @@ -368,10 +368,10 @@ reg_t kSetShowStyle(EngineState *s, int argc, reg_t *argv) {
const uint16 type = argv[0].toUint16();
reg_t planeObj = argv[1];
int16 seconds = argv[2].toSint16();
// NOTE: This value seems to indicate whether the transition is an
// “exit” transition (0) or an “enter” transition (-1) for fade
// transitions. For other types of transitions, it indicates a palette
// index value to use when filling the screen.
// This value indicates whether the transition is an "exit" transition (0)
// or an "enter" transition (-1) for fade transitions. For other types of
// transitions, it indicates a palette index value to use when filling the
// screen.
int16 back = argv[3].toSint16();
int16 priority = argv[4].toSint16();
int16 animate = argv[5].toSint16();
Expand Down Expand Up @@ -404,9 +404,8 @@ reg_t kSetShowStyle(EngineState *s, int argc, reg_t *argv) {
error("Illegal show style %d for plane %04x:%04x", type, PRINT_REG(planeObj));
}

// NOTE: The order of planeObj and showStyle are reversed
// because this is how SCI3 called the corresponding method
// on the KernelMgr
// The order of planeObj and showStyle are reversed because this is how
// SSCI3 called the corresponding method on the KernelMgr
g_sci->_gfxTransitions32->kernelSetShowStyle(argc, planeObj, (ShowStyleType)type, seconds, back, priority, animate, refFrame, pFadeArray, divisions, blackScreen);

return s->r_acc;
Expand Down Expand Up @@ -881,8 +880,8 @@ reg_t kSetScroll(EngineState *s, int argc, reg_t *argv) {
const int16 deltaY = argv[2].toSint16();
const GuiResourceId pictureId = argv[3].toUint16();
const bool animate = argv[4].toUint16();
// NOTE: speed was accepted as an argument, but then never actually used
// const int16 speed = argc > 5 ? (bool)argv[5].toSint16() : -1;
// argv[5] was some speed argument, but it was not actually used by SSCI, so
// we ignore it here
const bool mirrorX = argc > 6 ? (bool)argv[6].toUint16() : false;

g_sci->_gfxTransitions32->kernelSetScroll(plane, deltaX, deltaY, pictureId, animate, mirrorX);
Expand Down
56 changes: 24 additions & 32 deletions engines/sci/graphics/transitions32.cpp
View file
Expand Up @@ -126,7 +126,7 @@ void GfxTransitions32::processShowStyles() {
g_sci->_gfxFrameout->frameOut(true);
throttle();
}
} while(continueProcessing && doFrameOut);
} while (continueProcessing && doFrameOut);
}

void GfxTransitions32::processEffects(PlaneShowStyle &showStyle) {
Expand Down Expand Up @@ -249,14 +249,13 @@ void GfxTransitions32::kernelSetShowStyle(const uint16 argc, const reg_t planeOb

if (createNewEntry) {
entry = new PlaneShowStyle;
// NOTE: SCI2.1 engine tests if allocation returned a null pointer
// but then only avoids setting currentStep if this is so. Since
// this is a nonsensical approach, we do not do that here
// SSCI2.1 tests if allocation returned a null pointer but then only
// avoids setting currentStep if this is so. Since this nonsensical, we
// do not do that here
entry->currentStep = 0;
entry->processed = false;
entry->divisions = hasDivisions ? divisions : _defaultDivisions[type];
entry->plane = planeObj;
entry->fadeColorRangesCount = 0;

if (getSciVersion() < SCI_VERSION_2_1_MIDDLE) {
// for pixel dissolve
Expand All @@ -267,32 +266,26 @@ void GfxTransitions32::kernelSetShowStyle(const uint16 argc, const reg_t planeOb
entry->screenItems.clear();
entry->width = plane->_gameRect.width();
entry->height = plane->_gameRect.height();
} else {
entry->fadeColorRanges = nullptr;
if (hasFadeArray) {
// NOTE: SCI2.1mid engine does no check to verify that an array is
// successfully retrieved, and SegMan will cause a fatal error
// if we try to use a memory segment that is not an array
SciArray &table = *_segMan->lookupArray(pFadeArray);

uint32 rangeCount = table.size();
entry->fadeColorRangesCount = rangeCount;

// NOTE: SCI engine code always allocates memory even if the range
// table has no entries, but this does not really make sense, so
// we avoid the allocation call in this case
if (rangeCount > 0) {
entry->fadeColorRanges = new uint16[rangeCount];
for (size_t i = 0; i < rangeCount; ++i) {
entry->fadeColorRanges[i] = table.getAsInt16(i);
}
} else if (hasFadeArray) {
// SSCI2.1mid does no check to verify that an array is successfully
// retrieved
SciArray &table = *_segMan->lookupArray(pFadeArray);

const uint32 rangeCount = table.size();

// SSCI always allocates memory even if the range table has no
// entries, but this does not really make sense, so we avoid the
// allocation call in this case
if (rangeCount > 0) {
entry->fadeColorRanges.reserve(rangeCount);
for (uint32 i = 0; i < rangeCount; ++i) {
entry->fadeColorRanges.push_back(table.getAsInt16(i));
}
}
}
}

// NOTE: The original engine had no nullptr check and would just crash
// if it got to here
// SSCI had no nullptr check and would just crash if it got to here
if (entry == nullptr) {
error("Cannot edit non-existing ShowStyle entry");
}
Expand Down Expand Up @@ -397,10 +390,9 @@ ShowStyleList::iterator GfxTransitions32::deleteShowStyle(const ShowStyleList::i
break;
case kShowStyleFadeIn:
case kShowStyleFadeOut:
if (getSciVersion() > SCI_VERSION_2_1_EARLY && showStyle->fadeColorRangesCount > 0) {
delete[] showStyle->fadeColorRanges;
}
break;
// SSCI manually allocated the color ranges for fades and deleted that
// memory here, but we use a container so there is no extra cleanup
// needed
case kShowStyleNone:
case kShowStyleMorph:
case kShowStyleHShutterIn:
Expand Down Expand Up @@ -944,8 +936,8 @@ bool GfxTransitions32::processFade(const int8 direction, PlaneShowStyle &showSty
percent *= 100;
percent /= showStyle.divisions - 1;

if (showStyle.fadeColorRangesCount > 0) {
for (int i = 0, len = showStyle.fadeColorRangesCount; i < len; i += 2) {
if (showStyle.fadeColorRanges.size()) {
for (uint i = 0, len = showStyle.fadeColorRanges.size(); i < len; i += 2) {
g_sci->_gfxPalette32->setFade(percent, showStyle.fadeColorRanges[i], showStyle.fadeColorRanges[i + 1]);
}
} else {
Expand Down

0 comments on commit ff3503a

Please sign in to comment.