Skip to content
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.

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

quick fix for VS2019/Qt 5.15 build #1

Merged
merged 3 commits into from Aug 10, 2020
Merged

Conversation

LowLevelMahn
Copy link
Contributor

@LowLevelMahn LowLevelMahn commented Aug 3, 2020

app.pro: Visual Studio creates different lib names (its possible to seperate the changes with win32-msvc* { )
shapemodel.cpp/shapeview.cpp: Some sort of Mirosoft Problem #define + cmath does not work for M_PI, #define + math.h works for M_PI but then the std-namespaces is gone

@LowLevelMahn
Copy link
Contributor Author

fixed the QT_VERSION_STR Problem, added Ubuntu build instructions

Copy link
Owner

@dstien dstien left a comment

Choose a reason for hiding this comment

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

Thanks for looking into this! I added a few review questions as I'm honestly out of the loop in how Qt has developed in the past decade. The original win32 builds were made with MinGW and a custom built Qt under Linux, using Gentoo's crossdev. The goal was to try to reduce the bloated binary sizes, but the results were unsatisfactory, so I'm not eager to repeat that cumbersome exercise.

@@ -15,11 +15,13 @@
// along with this program; if not, write to the Free Software
// Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.

#include <windows.h>
Copy link
Owner

Choose a reason for hiding this comment

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

Why is this required? Should probably be wrapped in #if defined(Q_OS_WIN or similar to maintain portability.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the opengl header is not working when not including windows.h
first problematic line in windows gl.h: WINGDIAPI void APIENTRY glAccum (GLenum op, GLfloat value);

#if defined (_MSC_VER) // needed for glu.h if build with MSVC
#include <windows.h>
#endif

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://www.opengl.org//archives/resources/faq/technical/gettingstarted.htm

Also, note that you'll need to put an #include <windows.h> statement before the #include<GL/gl.h>. Microsoft requires system DLLs to use a specific calling convention that isn't the default calling convention for most Win32 C compilers, so they've annotated the OpenGL calls in gl.h with some macros that expand to nonstandard C syntax. This causes Microsoft's C compilers to use the system calling convention. One of the include files included by windows.h defines the macros.

@@ -318,7 +319,7 @@ void MainWindow::about()
QMessageBox::about(this, tr("About stressed"),
tr("<div align=\"center\"><h1>%1 %2</h1><br/>"
"%3<br/>"
"Using Qt "QT_VERSION_STR"/%5<br/>"
"Using Qt 5.15/%5<br/>"
Copy link
Owner

Choose a reason for hiding this comment

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

Can we still use QT_VERSION_STR as a numbered argument in the tr() format string? tr("Qt %1").arg(QT_VERSION_STR)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you can keep it nearly like that - its not allowed in C++11 to write "QT_VERSION_STR"
you need to write " QT_VERSION_STR ", (blank before and after ")
gcc 9.3 warns (see #2), VS2019 just don't compiles the line without the blanks around, i'll fixed that in my second change

@LowLevelMahn
Copy link
Contributor Author

can you also add this line to my README Linux Build extension:

sudo apt install libqt5opengl5-dev # needed for Mint 19.3, Ubuntu 18.04

@dstien dstien merged commit 933eff7 into dstien:master Aug 10, 2020
@dstien
Copy link
Owner

dstien commented Aug 10, 2020

Done and merged. Thanks!

dstien added a commit to dstien/stressed that referenced this pull request Jul 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants