Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion basicalgorithm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ QVector<Fixation> BasicAlgorithm::generateFixations() {
return fixations;
}

QVector <Fixation> BasicAlgorithm::generateSaccades() {
QVector <Saccade> BasicAlgorithm::generateSaccades() {
return saccades;
}

Expand Down
3 changes: 1 addition & 2 deletions basicalgorithm.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,7 @@ class BasicAlgorithm: public FixationAlgorithm {


QVector<Fixation> generateFixations() override;
QVector<Fixation> generateSaccades() override;

QVector<Saccade> generateSaccades() override;
QString generateFixationSettings() override;

private:
Expand Down
2 changes: 1 addition & 1 deletion controller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,7 @@ void Controller::generateFixationData(QVector<QString> tasks, QString algSetting
for(auto session_id : sessions) {
//std::cout << "?" << std::endl;
QVector<Fixation> session_fixations;
QVector<Fixation> session_saccades;
QVector<Saccade> session_saccades;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The variable session_saccades is declared but its value is never used. This is considered dead code and should be removed to improve code clarity.


QVector<QString> gaze_targets = idb.getGazeTargetsFromSession(session_id);
QString fixation_filter_settings;
Expand Down
8 changes: 8 additions & 0 deletions database.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,14 @@ void Database::insertGaze(QString event_time, QString session_id, QString calibr
sqlite3_exec(db,query.toStdString().c_str(),NULL,0,NULL);
}

//Inserts values/data into the appropriate attribute for the saccade and saccade_gaze tables
void Database::insertSaccade(QString saccade_id, QString fixation_run_id, QString start_time, QString end_time,
QString start_fixation, QString end_fixation, QString start_x, QString start_y,
QString end_x, QString end_y, QString amplitude, QString peak_velocity,
QString average_velocity, QString direction, QString duration){
QString query=QString("INSERT INTO saccade(saccade_id,fixation_run_id,start_time,end_time,start_fixation,end_fixation,start_x,start_y,end_x,end_y,amplitude,peak_velocity,average_velocity,direction,duration) VALUES(%1,%2,%3,%4,\"%5\",\"%6\",%7,%8,%9,%10,%11,%12,%13,%14,%15);").arg(saccade_id,fixation_run_id,start_time,end_time,start_fixation,end_fixation,start_x,start_y,end_x,end_y,amplitude,peak_velocity,average_velocity,direction,duration);
sqlite3_exec(db,query.toStdString().c_str(),NULL,0,NULL);
}
Comment on lines +189 to +195

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

This method of constructing SQL queries by string formatting is vulnerable to SQL injection. An attacker could provide malicious input for parameters like start_fixation to execute arbitrary SQL commands. Please use prepared statements with parameter binding to prevent this. For example, with sqlite3, you would use sqlite3_prepare_v2, sqlite3_bind_*, and sqlite3_step. This vulnerability also exists in insertSaccadeGaze and other insert methods in this file.

//issue 58
void Database::insertSaccadeGaze(QString saccade_id, QString event_time) {
QString query = QString("INSERT INTO saccade_gaze(saccade_id,event_time) VALUES(\"%1\",%2);").arg(saccade_id,event_time);
Expand Down
3 changes: 2 additions & 1 deletion database.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "gaze.h"

#include <iostream>
#include <QDebug>

class Database {
public:
Expand Down Expand Up @@ -78,9 +79,9 @@ class Database {

QVector<QVector<QString>> runFilterQuery(QString);
void executeLongUpdateQuery(QString);
sqlite3* db;

private:
sqlite3* db;
QString file_path;
bool open = false;
Comment on lines +82 to 86

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The database handle db has been moved from private to public. This breaks encapsulation and is a poor design choice. It allows any part of the application to directly access and manipulate the database, bypassing the Database class's interface. This can lead to maintenance issues and hard-to-track bugs. The db handle should remain a private member.

Suggested change
sqlite3* db;
private:
sqlite3* db;
QString file_path;
bool open = false;
private:
sqlite3* db;
QString file_path;
bool open = false;

};
Expand Down
62 changes: 62 additions & 0 deletions fixation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,3 +71,65 @@ void Fixation::calculateDatabaseFields() {
void Fixation::print() {
std::cout << fixation_event_time << "," << x << "," << y << "," << target.toUtf8().constData() << "," << source_file_line << "," << source_file_col << "," << token.toUtf8().constData() << "," << syntactic_category.toUtf8().constData() << "," << xpath.toUtf8().constData() << "," << left_pupil_diameter << "," << right_pupil_diameter << "," << duration << std::endl;
}


///////////////////////////////


Saccade::Saccade() {}

void Saccade::calculateDatabaseFields() {
long long start_time = -1, end_time = -1;
int gaze_count = 0;
std::map<QString,int> candidate_targets;
for(auto gaze : gaze_vec) {
if(!gaze.isValid()) { continue; }
if(fixation_event_time == 0 || fixation_event_time > gaze.event_time) {
fixation_event_time = gaze.event_time;
}

++gaze_count;

if(start_time == -1 || start_time > gaze.system_time) {
start_time = gaze.system_time;
}
if(end_time == -1 || end_time < gaze.system_time) {
end_time = gaze.system_time;
}


left_pupil_diameter += isnan(gaze.left_pupil_diameter) || gaze.left_pupil_diameter == -1.0 ? 0 : gaze.left_pupil_diameter;
right_pupil_diameter += isnan(gaze.right_pupil_diameter) || gaze.left_pupil_diameter == -1.0 ? 0 : gaze.right_pupil_diameter;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

There appears to be a copy-paste error here. When calculating right_pupil_diameter, the condition checks gaze.left_pupil_diameter == -1.0 instead of gaze.right_pupil_diameter == -1.0. This will lead to incorrect calculations for the right pupil diameter.

Suggested change
right_pupil_diameter += isnan(gaze.right_pupil_diameter) || gaze.left_pupil_diameter == -1.0 ? 0 : gaze.right_pupil_diameter;
right_pupil_diameter += isnan(gaze.right_pupil_diameter) || gaze.right_pupil_diameter == -1.0 ? 0 : gaze.right_pupil_diameter;


QString candidate_key = gaze.gaze_target + "\t";
candidate_key += (gaze.source_file_line == -1 ? QString("") : QString::number(gaze.source_file_line)) + "\t";
candidate_key += (gaze.source_file_col == -1 ? QString("") : QString::number(gaze.source_file_col)) + "\t";
candidate_key += gaze.source_token + "\t";
candidate_key += gaze.source_token_syntatic_context + "\t";
candidate_key += gaze.source_token_xpath + "\t";

if(candidate_targets.count(candidate_key) == 0) { candidate_targets.emplace(candidate_key,1); }
else { ++(candidate_targets.find(candidate_key)->second); }
}
std::pair<QString,int> most_frequent = std::make_pair(QString(""),0);
for(auto candidate = candidate_targets.begin(); candidate != candidate_targets.end(); ++candidate) {
if(most_frequent.first == "" || most_frequent.second < candidate->second) { most_frequent = *candidate; }
}

QStringList fields = most_frequent.first.split("\t");
target = fields[0] == "" ? "" : fields[0];
source_file_line = fields[1] == "" ? -1 : fields[1].toInt();
source_file_col = fields[2] == "" ? -1 : fields[2].toInt();
token = fields[3] == "" ? "" : fields[3];
syntactic_category = fields[4] == "" ? "" : fields[4];
xpath = fields[5] == "" ? "" : fields[5];

left_pupil_diameter = left_pupil_diameter / double(gaze_count);
right_pupil_diameter = right_pupil_diameter / double(gaze_count);
Comment on lines +127 to +128

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

If gaze_vec is empty or contains only invalid gazes, gaze_count will be 0. This will cause a division-by-zero error when calculating left_pupil_diameter and right_pupil_diameter. You should add a check to prevent this.

Suggested change
left_pupil_diameter = left_pupil_diameter / double(gaze_count);
right_pupil_diameter = right_pupil_diameter / double(gaze_count);
if (gaze_count > 0) {
left_pupil_diameter = left_pupil_diameter / double(gaze_count);
right_pupil_diameter = right_pupil_diameter / double(gaze_count);
} else {
left_pupil_diameter = 0;
right_pupil_diameter = 0;
}

duration = end_time - start_time;

}

void Saccade::print() {
std::cout << fixation_event_time << "," << x << "," << y << "," << target.toUtf8().constData() << "," << source_file_line << "," << source_file_col << "," << token.toUtf8().constData() << "," << syntactic_category.toUtf8().constData() << "," << xpath.toUtf8().constData() << "," << left_pupil_diameter << "," << right_pupil_diameter << "," << duration << std::endl;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The Saccade::print() method prints member variables x and y, but these are never initialized for a Saccade object. This will result in printing uninitialized (garbage) values. If they are not needed for Saccade, consider removing them from the print statement. If they are needed, they should be calculated, for example as the midpoint of the saccade.

}
21 changes: 21 additions & 0 deletions fixation.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,4 +55,25 @@ class Fixation {
long long end_time=-1;
};

class Saccade {
public:
Saccade();

void calculateDatabaseFields();

void print();

// This could maybe be moved to a set if we want to ignore duplicates
std::vector<Gaze> gaze_vec;
double x, y, left_pupil_diameter = 0, right_pupil_diameter = 0;
int source_file_line, source_file_col, duration = 0;
long long fixation_event_time = 0;
QString target = "", syntactic_category = "", token = "", xpath = "";

//issue 58 - I think we add these here?
int start_x, start_y, end_x, end_y;
double amplitude, peak_velocity, avg_velocity;
double direction;
};

#endif // FIXATION_H
3 changes: 2 additions & 1 deletion fixationalgorithm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ QVector<Fixation>& FixationAlgorithm::getFixations() {
return fixations;
}

QVector<Fixation>& FixationAlgorithm::getSaccades(){

QVector<Saccade>& FixationAlgorithm::getSaccades(){
return saccades;
}
11 changes: 7 additions & 4 deletions fixationalgorithm.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,9 @@ class FixationAlgorithm
QVector<Fixation>& getFixations();

//issue 58 - Adding virtual function for generating saccades
virtual QVector<Fixation> generateSaccades()=0;
QVector<Fixation>& getSaccades();
virtual QVector<Saccade> generateSaccades()=0;
QVector<Saccade>& getSaccades();


protected:
virtual Fixation computeFixationEstimate(QVector<Gaze>)=0;
Expand All @@ -43,8 +44,10 @@ class FixationAlgorithm
QVector<Fixation> fixations;

//issue 58 - a vector to store our saccades
QVector<Fixation> saccades;
Fixation computeSaccadeEstimate(QVector<Gaze>);
//Also, not sure if we'll need a saccade estimate
QVector<Saccade> saccades;
Saccade computeSaccadeEstimate(QVector<Gaze>);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The function computeSaccadeEstimate is declared as a non-virtual member function in the base class FixationAlgorithm, but it is not implemented here. This will cause a linker error if it's ever called on a FixationAlgorithm pointer/reference that points to a derived class that doesn't implement it (like BasicAlgorithm). To enforce that derived classes provide an implementation, you should declare it as a pure virtual function, similar to computeFixationEstimate.

Suggested change
Saccade computeSaccadeEstimate(QVector<Gaze>);
virtual Saccade computeSaccadeEstimate(QVector<Gaze>)=0;


};

#endif // FIXATIONALGORITHM_H
5 changes: 3 additions & 2 deletions gaze.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,16 @@ class Gaze {
// void swap(Gaze&);

bool isValid();

void print();

int db_id = -1;

int left_validation, right_validation,
source_file_line = -1, source_file_col = -1;
long long event_time, system_time;
double x, y, left_pupil_diameter, right_pupil_diameter;
QString gaze_target = "", gaze_target_type = "",
source_token = "", source_token_xpath = "", source_token_syntatic_context = "";
source_token = "", source_token_xpath = "", source_token_syntatic_context = "";

//friend std::ostream& operator<<(std::ostream&,const Gaze&);

Expand Down
3 changes: 2 additions & 1 deletion idtalgorithm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,8 @@ QVector<Fixation> IDTAlgorithm::generateFixations() {
return fixations;
}

QVector<Fixation> IDTAlgorithm::generateSaccades(){

QVector<Saccade> IDTAlgorithm::generateSaccades(){
return saccades;
}

Expand Down
2 changes: 1 addition & 1 deletion idtalgorithm.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class IDTAlgorithm : public FixationAlgorithm{
QVector<Fixation> generateFixations() override;
QString generateFixationSettings() override;

QVector<Fixation> generateSaccades() override;
QVector<Saccade> generateSaccades() override;
private:
Fixation computeFixationEstimate(QVector<Gaze>) override;

Expand Down
24 changes: 13 additions & 11 deletions ivtalgorithm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@
//Helper Functions
double calculateGazeVelocity(double x1, double y1, double x2, double y2) {
double vx = x1 - x2,
vy = y1 - y2;
vy = y1 - y2;
double v = sqrt((vx*vx)+(vy*vy));
return v;
}

//issue 58 - additonal helper functions for saccade attributes (peak velocity, average velocity, direction)

double calculateAverageVelocity(QVector<Gaze> gazes ){
double totalVelocity= 0;
double averageVelocity=0;
Expand All @@ -40,6 +40,7 @@ double calculatePeakVelocity(QVector<Gaze> gazes){
return peakVelocity;
}


double calculateGazeDirection(QVector <Gaze> gazes){
double dx=0;
double dy=0;
Expand Down Expand Up @@ -107,7 +108,7 @@ QVector<Fixation> IVTAlgorithm::generateFixations() {
}

//issue 58 - creating a separate vector for saccades
QVector<Fixation> IVTAlgorithm::generateSaccades() {
QVector<Saccade> IVTAlgorithm::generateSaccades() {
std::vector<double> velocity_vector;
velocity_vector.push_back(0);

Expand All @@ -132,6 +133,7 @@ QVector<Fixation> IVTAlgorithm::generateSaccades() {
}

}

// Step 4 - Filter the saccade groupings-needs computeSaccadeEstimate to be implemented
QVector<Gaze> tmp;

Expand All @@ -141,13 +143,13 @@ QVector<Fixation> IVTAlgorithm::generateSaccades() {
}
else if(saccade_groups[i].second == saccade_groups[i-1].second) {
tmp.push_back(saccade_groups[i].first);
Fixation sacc = computeSaccadeEstimate(tmp);
if(sacc.start_x > -1) { saccades.push_back(sacc); }
Saccade sacc = computeSaccadeEstimate(tmp);
if(sacc.start_x>-1) { saccades.push_back(sacc); }
tmp.clear();
}
else {
Fixation sacc = computeSaccadeEstimate(tmp);
if(sacc.start_x > -1) { saccades.push_back(sacc); }
Saccade sacc = computeSaccadeEstimate(tmp);
if(sacc.start_x>-1) { saccades.push_back(sacc); }
tmp.clear();
}
}
Expand All @@ -158,7 +160,7 @@ Fixation IVTAlgorithm::computeFixationEstimate(QVector<Gaze> fixation_points) {
Fixation fixation;

double x_total = 0,
y_total = 0;
y_total = 0;
for(auto point : fixation_points) {
x_total += point.x;
y_total += point.y;
Expand All @@ -181,14 +183,14 @@ Fixation IVTAlgorithm::computeFixationEstimate(QVector<Gaze> fixation_points) {
}

//convert to saccadeEstimate? Will be different from above, will calculate start x and start y
Fixation IVTAlgorithm::computeSaccadeEstimate(QVector<Gaze> saccade_points) {
Fixation saccade;
Saccade IVTAlgorithm::computeSaccadeEstimate(QVector<Gaze> saccade_points) {
Saccade saccade;

double dx=0;
double dy=0;

for(auto point : saccade_points) {
saccade.start_x=saccade_points.first().x;
saccade.start_x=saccade_points.first().x;
saccade.start_y=saccade_points.first().y;
saccade.end_x=saccade_points.last().x;
saccade.end_y=saccade_points.last().y;
Expand Down
10 changes: 7 additions & 3 deletions ivtalgorithm.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
#define IVTALGORITHM_H

#include "fixationalgorithm.h"
#include "math.h"
#include "database.h"

class IVTAlgorithm: public FixationAlgorithm {
public:
Expand All @@ -22,19 +24,21 @@ class IVTAlgorithm: public FixationAlgorithm {
QVector<Fixation> generateFixations() override;
QString generateFixationSettings() override;


//issue 58
QVector<Fixation> generateSaccades() override;
QVector<Saccade> generateSaccades() override;

//issue 58-Connor
//issue 58-Conner
//QVector<Gaze> gaze_vec;

private:
Fixation computeFixationEstimate(QVector<Gaze>) override;
int velocity_threshold;
int duration_ms;

Database db;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The Database db; member is declared but it is never used within the IVTAlgorithm class. This is dead code and should be removed to improve clarity and reduce the object's memory footprint.

//issue 58
Fixation computeSaccadeEstimate(QVector<Gaze>);
Saccade computeSaccadeEstimate(QVector<Gaze>);
};

#endif // IVTALGORITHM_H