Skip to content

Commit

Permalink
#5746: Getting the transformation on patches to be correct, undoable …
Browse files Browse the repository at this point in the history
…and live-updating the views seems to be very complicated. Its freezeTransform() behaviour is quite different from faces, which might be an indication of one or more design issues. Patch::controlPointsChanged() does way too much, including revertTransform.
  • Loading branch information
codereader committed Sep 18, 2021
1 parent e87847b commit b1cbece
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 7 deletions.
10 changes: 10 additions & 0 deletions include/ipatch.h
Expand Up @@ -215,8 +215,18 @@ class IPatch
// Reverts any transform that has been applied since the last time freezeTransform() was called
virtual void revertTransform() = 0;

// Returns the transformable control point (not the one returned by ctrlAt) - this one is
// part of the "transformed" working set of the patch which will either be saved on freezeTransform()
// or discarded on revertTransform().
virtual PatchControl& getTransformedCtrlAt(std::size_t row, std::size_t col) = 0;

// Promotes the current transformed state to the new base state
virtual void freezeTransform() = 0;

// Updates the patch tesselation based on the transformed set of control vertices
// Setting force to true will update the tesselation even if controlPointsChanged()
// hasn't been called in the meantime.
virtual void updateTesselation(bool force = false) = 0;
};

namespace patch
Expand Down
9 changes: 7 additions & 2 deletions radiantcore/patch/Patch.cpp
Expand Up @@ -406,6 +406,11 @@ const PatchControl& Patch::ctrlAt(std::size_t row, std::size_t col) const {
return _ctrl[row*_width+col];
}

PatchControl& Patch::getTransformedCtrlAt(std::size_t row, std::size_t col)
{
return _ctrlTransformed[row * _width + col];
}

// called just before an action to save the undo state
void Patch::undoSave()
{
Expand Down Expand Up @@ -518,10 +523,10 @@ bool Patch::isDegenerate() const {
return true;
}

void Patch::updateTesselation()
void Patch::updateTesselation(bool force)
{
// Only do something if the tesselation has actually changed
if (!_tesselationChanged) return;
if (!_tesselationChanged && !force) return;

_tesselationChanged = false;

Expand Down
6 changes: 4 additions & 2 deletions radiantcore/patch/Patch.h
Expand Up @@ -218,6 +218,8 @@ class Patch :
// The same as above just for const
const PatchControl& ctrlAt(std::size_t row, std::size_t col) const override;

PatchControl& getTransformedCtrlAt(std::size_t row, std::size_t col) override;

/** greebo: Inserts two columns before and after the column with index <colIndex>.
* Throws an GenericPatchException if an error occurs.
*/
Expand Down Expand Up @@ -359,12 +361,12 @@ class Patch :
// Static signal holder, signal is emitted after any patch texture has changed
static sigc::signal<void>& signal_patchTextureChanged();

void updateTesselation(bool force = false);

private:
// This notifies the surfaceinspector/patchinspector about the texture change
void textureChanged();

void updateTesselation();

// greebo: checks, if the shader name is valid
void check_shader();

Expand Down
13 changes: 10 additions & 3 deletions radiantcore/selection/textool/PatchNode.h
@@ -1,6 +1,7 @@
#pragma once

#include "ipatch.h"
#include "itextstream.h"
#include "NodeBase.h"

namespace textool
Expand All @@ -20,13 +21,15 @@ class PatchNode :

void beginTransformation() override
{
// We call undoSave() here for consistency, but technically it's too early -
// the undo operation hasn't started yet
_patch.undoSave();
}

void revertTransformation() override
{
_patch.revertTransform();
_patch.controlPointsChanged();
_patch.updateTesselation();
}

void applyTransformToSelected(const Matrix3& transform) override
Expand All @@ -36,11 +39,15 @@ class PatchNode :
vertex.texcoord = transform * vertex.texcoord;
});

_patch.controlPointsChanged();
// We have to force the patch to update its tesselation since
// modifying the "transformed" control point set won't trigger this
_patch.updateTesselation(true);
}

void commitTransformation() override
{
// Patch::freezeTransform will call undoSave() before overwriting
// the control point set with the transformed ones
_patch.freezeTransform();
}

Expand Down Expand Up @@ -118,7 +125,7 @@ class PatchNode :
{
for (std::size_t row = 0; row < _patch.getHeight(); ++row)
{
functor(_patch.ctrlAt(row, col));
functor(_patch.getTransformedCtrlAt(row, col));
}
}
}
Expand Down

0 comments on commit b1cbece

Please sign in to comment.