Skip to content

Commit 504ea0c

Browse files
committed
Make Keyframe::Values and Keyframe::Points vectors private
The Values vector should only be accessed from the outside through the GetValue() function. The Points vector should only be accessed using the AddPoint(), RemovePoint(), .. functions. This helps maintain internal invariants (e.g. keeping Points sorted) and allows for future removal / lazy evaluation of Values. The size() of the vectors had been accessed from various parts of the code; the GetLength() (for Values) and GetCount() (for Points) member functions provide access to this information and are already part of the public API.
1 parent cb55741 commit 504ea0c

File tree

4 files changed

+15
-15
lines changed

4 files changed

+15
-15
lines changed

include/KeyFrame.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,8 @@ namespace openshot {
6565
private:
6666
bool needs_update;
6767
double FactorialLookup[4];
68+
std::vector<Point> Points; ///< Vector of all Points
69+
std::vector<Coordinate> Values; ///< Vector of all Values (i.e. the processed coordinates from the curve)
6870

6971
// Process an individual segment
7072
void ProcessSegment(int Segment, Point p1, Point p2);
@@ -82,8 +84,6 @@ namespace openshot {
8284
double Bernstein(int64_t n, int64_t i, double t);
8385

8486
public:
85-
std::vector<Point> Points; ///< Vector of all Points
86-
std::vector<Coordinate> Values; ///< Vector of all Values (i.e. the processed coordinates from the curve)
8787

8888
/// Default constructor for the Keyframe class
8989
Keyframe();

src/Clip.cpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -109,10 +109,10 @@ void Clip::init_settings()
109109
// Init reader's rotation (if any)
110110
void Clip::init_reader_rotation() {
111111
// Only init rotation from reader when needed
112-
if (rotation.Points.size() > 1)
112+
if (rotation.GetCount() > 1)
113113
// Do nothing if more than 1 rotation Point
114114
return;
115-
else if (rotation.Points.size() == 1 && rotation.GetValue(1) != 0.0)
115+
else if (rotation.GetCount() == 1 && rotation.GetValue(1) != 0.0)
116116
// Do nothing if 1 Point, and it's not the default value
117117
return;
118118

@@ -273,7 +273,7 @@ void Clip::Close()
273273
float Clip::End()
274274
{
275275
// if a time curve is present, use its length
276-
if (time.Points.size() > 1)
276+
if (time.GetCount() > 1)
277277
{
278278
// Determine the FPS fo this clip
279279
float fps = 24.0;
@@ -314,8 +314,8 @@ std::shared_ptr<Frame> Clip::GetFrame(int64_t requested_frame)
314314
// Is a time map detected
315315
int64_t new_frame_number = requested_frame;
316316
int64_t time_mapped_number = adjust_frame_number_minimum(time.GetLong(requested_frame));
317-
if (time.Values.size() > 1)
318-
new_frame_number = time_mapped_number;
317+
if (time.GetLength() > 1)
318+
new_frame_number = time_mapped_number;
319319

320320
// Now that we have re-mapped what frame number is needed, go and get the frame pointer
321321
std::shared_ptr<Frame> original_frame;
@@ -397,7 +397,7 @@ void Clip::get_time_mapped_frame(std::shared_ptr<Frame> frame, int64_t frame_num
397397
throw ReaderClosed("No Reader has been initialized for this Clip. Call Reader(*reader) before calling this method.");
398398

399399
// Check for a valid time map curve
400-
if (time.Values.size() > 1)
400+
if (time.GetLength() > 1)
401401
{
402402
const GenericScopedLock<juce::CriticalSection> lock(getFrameCriticalSection);
403403

src/Timeline.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -824,7 +824,7 @@ std::shared_ptr<Frame> Timeline::GetFrame(int64_t requested_frame)
824824
ZmqLogger::Instance()->AppendDebugMethod("Timeline::GetFrame (Adding solid color)", "frame_number", frame_number, "info.width", info.width, "info.height", info.height);
825825

826826
// Add Background Color to 1st layer (if animated or not black)
827-
if ((color.red.Points.size() > 1 || color.green.Points.size() > 1 || color.blue.Points.size() > 1) ||
827+
if ((color.red.GetCount() > 1 || color.green.GetCount() > 1 || color.blue.GetCount() > 1) ||
828828
(color.red.GetValue(frame_number) != 0.0 || color.green.GetValue(frame_number) != 0.0 || color.blue.GetValue(frame_number) != 0.0))
829829
new_frame->AddColor(Settings::Instance()->MAX_WIDTH, Settings::Instance()->MAX_HEIGHT, color.GetColorHex(frame_number));
830830

tests/KeyFrame_Tests.cpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ TEST(Keyframe_GetPoint_With_1_Points)
5151
k1.AddPoint(openshot::Point(2,3));
5252

5353
CHECK_THROW(k1.GetPoint(-1), OutOfBoundsPoint);
54-
CHECK_EQUAL(1, k1.Points.size());
54+
CHECK_EQUAL(1, k1.GetCount());
5555
CHECK_CLOSE(2.0f, k1.GetPoint(0).co.X, 0.00001);
5656
CHECK_CLOSE(3.0f, k1.GetPoint(0).co.Y, 0.00001);
5757
CHECK_THROW(k1.GetPoint(1), OutOfBoundsPoint);
@@ -98,7 +98,7 @@ TEST(Keyframe_GetValue_For_Bezier_Curve_2_Points)
9898
CHECK_CLOSE(3.81602f, kf.GetValue(40), 0.0001);
9999
CHECK_CLOSE(4.0f, kf.GetValue(50), 0.0001);
100100
// Check the expected number of values
101-
CHECK_EQUAL(kf.Values.size(), 51);
101+
CHECK_EQUAL(kf.GetLength(), 51);
102102
}
103103

104104
TEST(Keyframe_GetValue_For_Bezier_Curve_5_Points_40_Percent_Handle)
@@ -121,7 +121,7 @@ TEST(Keyframe_GetValue_For_Bezier_Curve_5_Points_40_Percent_Handle)
121121
CHECK_CLOSE(1.77569f, kf.GetValue(177), 0.0001);
122122
CHECK_CLOSE(3.0f, kf.GetValue(200), 0.0001);
123123
// Check the expected number of values
124-
CHECK_EQUAL(kf.Values.size(), 201);
124+
CHECK_EQUAL(kf.GetLength(), 201);
125125
}
126126

127127
TEST(Keyframe_GetValue_For_Bezier_Curve_5_Points_25_Percent_Handle)
@@ -144,7 +144,7 @@ TEST(Keyframe_GetValue_For_Bezier_Curve_5_Points_25_Percent_Handle)
144144
CHECK_CLOSE(1.77569f, kf.GetValue(177), 0.0001);
145145
CHECK_CLOSE(3.0f, kf.GetValue(200), 0.0001);
146146
// Check the expected number of values
147-
CHECK_EQUAL(kf.Values.size(), 201);
147+
CHECK_EQUAL(kf.GetLength(), 201);
148148
}
149149

150150
TEST(Keyframe_GetValue_For_Linear_Curve_3_Points)
@@ -164,7 +164,7 @@ TEST(Keyframe_GetValue_For_Linear_Curve_3_Points)
164164
CHECK_CLOSE(4.4f, kf.GetValue(40), 0.0001);
165165
CHECK_CLOSE(2.0f, kf.GetValue(50), 0.0001);
166166
// Check the expected number of values
167-
CHECK_EQUAL(kf.Values.size(), 51);
167+
CHECK_EQUAL(kf.GetLength(), 51);
168168
}
169169

170170
TEST(Keyframe_GetValue_For_Constant_Curve_3_Points)
@@ -185,7 +185,7 @@ TEST(Keyframe_GetValue_For_Constant_Curve_3_Points)
185185
CHECK_CLOSE(8.0f, kf.GetValue(49), 0.0001);
186186
CHECK_CLOSE(2.0f, kf.GetValue(50), 0.0001);
187187
// Check the expected number of values
188-
CHECK_EQUAL(kf.Values.size(), 51);
188+
CHECK_EQUAL(kf.GetLength(), 51);
189189
}
190190

191191
TEST(Keyframe_Check_Direction_and_Repeat_Fractions)

0 commit comments

Comments
 (0)