Skip to content

Use Qt for directory functions #175

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Cambloid wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Use Qt for directory functions #175

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
14006f5
Use Qt for directory functions
Mar 16, 2024
fa71fdf
Changes
Mar 18, 2024
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
10 changes: 8 additions & 2 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -921,8 +921,13 @@ $(INSTALLDIR)/radiant.$(EXE): \
libprofile.$(A) \
libquickhull.$(A) \
libxmllib.$(A) \
libos.$(A) \
$(if $(findstring Win32,$(OS)),icons/radiant.o,) \

libos.$(A): CPPFLAGS_EXTRA := $(CPPFLAGS_QTCORE) -Ilibs -Iinclude
libos.$(A): \
libs/os/dir.o \

libfilematch.$(A): CPPFLAGS_EXTRA := -Ilibs
libfilematch.$(A): \
libs/filematch.o \
Expand Down Expand Up @@ -1072,13 +1077,14 @@ $(INSTALLDIR)/modules/shaders.$(DLL): \
plugins/shaders/shaders.o \
libcommandlib.$(A) \

$(INSTALLDIR)/modules/vfspk3.$(DLL): LIBS_EXTRA := $(LIBS_GLIB)
$(INSTALLDIR)/modules/vfspk3.$(DLL): CPPFLAGS_EXTRA := $(CPPFLAGS_GLIB) -Ilibs -Iinclude
$(INSTALLDIR)/modules/vfspk3.$(DLL): LIBS_EXTRA := $(LIBS_GLIB) $(LIBS_QTCORE)
$(INSTALLDIR)/modules/vfspk3.$(DLL): CPPFLAGS_EXTRA := $(CPPFLAGS_GLIB) $(CPPFLAGS_QTCORE) -Ilibs -Iinclude
$(INSTALLDIR)/modules/vfspk3.$(DLL): \
plugins/vfspk3/archive.o \
plugins/vfspk3/vfs.o \
plugins/vfspk3/vfspk3.o \
libfilematch.$(A) \
libos.$(A) \

$(INSTALLDIR)/plugins/bobtoolz.$(DLL): LIBS_EXTRA := $(LIBS_GLIB) $(LIBS_QTCORE) $(LIBS_QTGUI) $(LIBS_QTWIDGETS)
$(INSTALLDIR)/plugins/bobtoolz.$(DLL): CPPFLAGS_EXTRA := $(CPPFLAGS_GLIB) $(CPPFLAGS_QTCORE) $(CPPFLAGS_QTGUI) $(CPPFLAGS_QTWIDGETS) -Ilibs -Iinclude
Expand Down
40 changes: 40 additions & 0 deletions libs/os/dir.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
#include "dir.h"

#include <QDir>
#include <QString>
#include <QStringList>
#include <QFileInfo>

Directory::Directory(const char* name) {
this->dir = new QDir(name);
}

bool Directory::good() {
if(this->dir) {
QDir* dir_instance = reinterpret_cast<QDir*>(this->dir);
return dir_instance->exists();
}
return false;
}

void Directory::close() {
if(this->dir) {
QDir* dir_instance = reinterpret_cast<QDir*>(this->dir);
delete dir_instance;
this->dir = nullptr;
}
}

const char* Directory::read_and_increment(){
if(this->dir) {
QDir* dir_instance = reinterpret_cast<QDir*>(this->dir);
QStringList entryList = dir_instance->entryList(QDir::Dirs | QDir::Files | QDir::NoDotAndDotDot);
Copy link
Owner

@Garux Garux Mar 17, 2024

Choose a reason for hiding this comment

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

This is srs perf penalty. E.g. with 1k entries in a browsed folder level at least 1M allocations will occur.

Copy link
Author

@Cambloid Cambloid Mar 18, 2024

Choose a reason for hiding this comment

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

changed

if(this->entry_idx < entryList.size()) {
QString file_path = entryList.at(this->entry_idx);
QString file_name = QFileInfo(file_path).fileName();
this->entry_idx++;
return file_name.toStdString().c_str();
Copy link
Owner

@Garux Garux Mar 17, 2024

Choose a reason for hiding this comment

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

.toLatin1() is more correct for this sort of conversions.
And we are returning pointer to temp local variable here ⚰️

Copy link
Author

@Cambloid Cambloid Mar 18, 2024

Choose a reason for hiding this comment

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

changed

Copy link
Author

@Cambloid Cambloid Mar 18, 2024

Choose a reason for hiding this comment

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

The reason why glib was removed is because qt provides directory management therefore glib seems to be redundant.

Copy link
Author

@Cambloid Cambloid Mar 18, 2024

Choose a reason for hiding this comment

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

I would love to know your take on this, this project already heavily uses qt why not use qt core for internal stuff as well.

Copy link
Owner

@Garux Garux Mar 20, 2024

Choose a reason for hiding this comment

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

Qt is used almost exclusively for UI.
Ic no positive effects of this change. It's not even removing glib dependency, which is used in other places.
While it adds dependency on big juicy framework, which may be unwelcome in case of some potential porting. Imagine doing Gtk->Qt port if not just UI was dependent on Gtk.
Also performance and behavior compatibility of this change are questionable.

}
}
return nullptr;
}
40 changes: 28 additions & 12 deletions libs/os/dir.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,24 +24,40 @@
/// \file
/// \brief OS directory-listing object.

#include <glib.h>

typedef GDir Directory;

inline bool directory_good( Directory* directory ){
return directory != 0;
class Directory {
private:
int entry_idx = 0;
void* dir;

public:
Directory(const char* name);
bool good();
void close();
const char* read_and_increment();
};

inline bool directory_good(Directory* directory) {
if(directory) {
return directory->good();
}
return false;
}

inline Directory* directory_open( const char* name ){
return g_dir_open( name, 0, 0 );
inline Directory* directory_open(const char* name){
return new Directory(name);
}

inline void directory_close( Directory* directory ){
g_dir_close( directory );
inline void directory_close(Directory* directory){
if(directory) {
directory->close();
}
}

inline const char* directory_read_and_increment( Directory* directory ){
return g_dir_read_name( directory );
inline const char* directory_read_and_increment(Directory* directory) {
if(directory) {
return directory->read_and_increment();
}
return nullptr;
}

template<typename Functor>
Expand Down