This commit performs three related cleanups:
* The slvs library no longer uses explicit platform initialization
(which drags in the side effects of InitPlatform that are not
desirable in a library). Instead, it just ensures that it has
the temporary heap, which is what it was callingInitPlatform for.
* InitPlatform is simplified and moved to platform.cpp, next to
other path related functions.
* InitPlatform is renamed to InitCli and is called from InitGui
implementations. GUI toolkits sometimes have options they use
internally (that's the case for for GTK and Cocoa at least),
and we shouldn't try to parse those as a file to open.
Historically SolveSpace used its own heap on Windows since it gave
better control and debugging options, but a lot of development these
days happens on Linux, where that heap was a stub around malloc/free,
and also Windows debugging tools got a lot better.
In terms of immediate benefit, this commit fixes heap corruption
on Windows introduced in commits b4e1ce44 and 47e82798, caused
by allocating with HEAP_NO_SERIALIZE in parallel from OpenMP threads.
Without HEAP_NO_SERIALIZE there's no performance benefit to keeping
our own heap, either.
The vl() function is also removed because for development there are
better tools now, and the only place where it was permanently called
from became a no-op, since temporary heap always validates after
FreeAllTemporary() recreates it.
A warning found with /W4 by MSVC 2019 (Microsoft (R) C/C++ Optimizing Compiler Version 19.24.28314)
is an actual bug. How does the SpaceMouse (I do not have one) work at all when the global `hSpaceWareDriverClass` is NULL?!
.\src\platform\guiwin.cpp(1392,34): warning C4459: declaration of 'hSpaceWareDriverClass' hides global declaration
.\src\platform\guiwin.cpp(1389,13): message : see declaration of 'SolveSpace::Platform::hSpaceWareDriverClass'
Found with /W4 by MSVC 2019 (Microsoft (R) C/C++ Optimizing Compiler Version 19.24.28314)
A bunch of implicit casts 'double' to 'float' and one 'int64_t' to 'unsigned'.
.\src\platform\guiwin.cpp(1237): warning C4701: potentially uninitialized local variable 'cursorName' used
.\src\platform\guiwin.cpp(1237): warning C4703: potentially uninitialized local pointer variable 'cursorName' used
.\src\solvespace.cpp(805,30): warning C4456: declaration of 'gs' hides previous local declaration
.\src\solvespace.cpp(715,17): message : see declaration of 'gs'
.\src\solvespace.cpp(849,47): warning C4456: declaration of 'e' hides previous local declaration
.\src\solvespace.cpp(847,29): message : see declaration of 'e'
.\src\render\render.h(288,51): warning C4458: declaration of 'camera' hides class member
.\src\render\render.h(271,17): message : see declaration of 'SolveSpace::SurfaceRenderer::camera'
.\src\render\render.h(289,57): warning C4458: declaration of 'lighting' hides class member
.\src\render\render.h(272,17): message : see declaration of 'SolveSpace::SurfaceRenderer::lighting'
Since Catalina or earlier this no longer causes artifacts when Cocoa
controls are overlaid on a GL layer. Conversely, offscreen rendering
is very slow, especially on HiDPI screens.
Co-Authored-By: Koen Schmeets <hello@koenschmeets.nl>
When drawing the graphics window, we flush it twice: once to draw
the geometry, and another time to draw the UI overlay (toolbar,
selection marquee, and FPS counter). Calling glFinish() each time
is (on most platforms) just pointlessly slow, but on macOS Catalina,
without offscreen rendering, it causes the toolbar to flicker.
Instead of calling glFinish() twice per frame in that case, call
glFlush() twice and then glFinish() once we really are done.
Its only use was in a context where it was completely equivalent to
MemFree, so just use that instead, and keep the temporary heap as
purely an arena allocator, that could use something like bump
pointer.
This is currently necessary to get repeatable results when exporting
assemblies as a part of a batch process, since the mesh geometry in
imported files is not regenerated for export.
By setting WINVER=0x0501 (Windows XP) in CMakeLists.txt and adding a few
missing defines in guiwin.cpp and configuring OPENGL=1 in CMake
Solvespace (3.0~25b6eba1) compiles and works perfectly on Windows XP.
Tested with MinGW GCC-6.3.0-1
This fixes an elusive GTK issue where tooltips would be spuriously
displayed, and makes tooltips behave nicer on Windows.
Unfortunately the macOS code is unchanged as the macOS tooltip
implementation seems seriously broken in ways I do not understand.
After this commit, if the target system does have modern OpenGL
drivers installed, ANGLE is configured to use them, bypassing most
translation (shaders still have to be translated from ESSL to GLSL).
If there are no OpenGL drivers, such as if the graphics drivers were
installed via Windows Update, DirectX translation is still used. This
results in a very noticeable startup delay and minor performance
degradation.
In addition it is no longer necessary to build with -DOPENGL=1 to be
able to run the binary in wine; everything works out of the box.
Before, wine's incomplete HLSL translator would crash.
This change required renaming the variable `texture` in shaders,
since it shadows the Core GLSL function with the same name, and ANGLE
translates texture2D() calls to texture() calls.
It is not clear why this code was added (I don't remember) and
the normal parent-child relationship should be sufficient for
the task of keeping property browser on top of the main window.
With SetWindowPos(hwnd, HWND_TOPMOST) though, the property browser
window stays on top of *anything*, even if the user switches to
an entirely different application.
Ubuntu 18.04 uses GTKMM 3.22.2-2, which doesn't support native file chooser.
Commit bc3e09edbf checks if native file chooser
is available, but the result is overridden with a hardcoded define,
probably for debugging.
Removing the debugging code fixes build on Ubuntu 18.04.
We plan to use flatbuffers in the future for the next generation of
the .slvs file format, so flatbuffers are built unconditionally; and
the Q3DO exporter itself is tiny.
This serves two purposes.
First, we want to (some day) convert these messages into a less
obtrustive form, something like toaster notifications, such that they
don't interrupt workflow as harshly. That would, of course, be
nonblocking.
Second, some platforms, like Emscripten, do not support nested event
loops, and it's not possible to display a modal dialog on them
synchronously.
When making this commit, I've reviewed all Error() and Message()
calls to ensure that only some of the following is true for all
of them:
* The call is followed a break or return statement that exits
an UI entry point (e.g. an MenuX function);
* The call is followed by cleanup (in fact, in this case the new
behavior is better, since even with a synchronous modal dialog
we have to be reentrant);
* The message is an informational message only and nothing
unexpected will happen if the operation proceeds in background.
In general, all Error() calls already satisfied the above conditions,
although in some cases I changed control flow aroudn them to more
clearly show that. The Message() calls that didn't satisfy these
conditions were reworked into an asynchronous form.
There are three explicit RunModal() calls left that need to be
reworked into an async form.
We have a lot of classes with virtual functions but no virtual
destructor, mostly under render/. While this is not a problem
due to how our hierarchy is structured, some versions of clang
warn about this on the delete statement inside shared_ptr.
We could add a virtual destructor, but adding final qualifiers
expresses intent better, is generally more efficient (since it allows
devirtualizing most virtual calls in render/), and solves
the potential problem clang is warning us about.
We currently support MSVC 2013, and MSVC 2013 has weird bugs around
std::unique_ptr; the one we hit is Connect ID 858243. You can't
actually open the bug report anymore because Microsoft has shut down
Microsoft Connect. We probably shouldn't support a compiler so old
its bugtracker doesn't exist anymore, but there isn't any very good
reason to use unique_ptr for TimerRef either, so let's change that
for the time being.
This commit removes Platform::Window::Redraw function, and rewrites
its uses to run on timer events. Most UI toolkits have obscure issues
with recursive event handling loops, and Emscripten is purely event-
driven and cannot handle imperative redraws at all.
As a part of this change, the Platform::Timer::WindUp function
is split into three to make the interpretation of its argument
less magical. The new functions are RunAfter (a regular timeout,
setTimeout in browser terms), RunAfterNextFrame (an animation
request, requestAnimationFrame in browser terms), and
RunAfterProcessingEvents (a request to run something after all
events for the current frame are processed, used for coalescing
expensive operations in face of input event queues).
This commit changes two uses of Redraw(): the AnimateOnto() and
ScreenStepDimGo() functions. The latter was actually broken in that
on small sketches, it would run very quickly and not animate
the dimension change at all; this has been fixed.
While we're at it, get rid of unused Platform::Window::NativePtr
function as well.
This is to address MSVC warnings.
This commit changes a few configuration fields to use double instead
of float. There doesn't seem to be any reason these use float except
for the legacy Windows code using float for saved configuration.
Changing their type to double improves consistency.
This commit merges all ad-hoc file dialog code, such as the feature
where dialogs remember last location and format, and exposes it
through a common interface.
This commit also significantly improves Gtk dialog handling code.
This commit changes the awfully specific code for dialogs with
messages duplicated three times to go through a generic interface.
It also fixes some issues with the way translated messages
were parameterized.
This commit removes the custom message dialog box used on Windows,
for several reasons. First, it was the last element not respecting
HiDPI displays. Second, other OSes do not easily provide this much
control over rendering default message boxes, and both Gnome and
macOS frown upon non-standard renderings such as those; so the custom
rendering was already not used on the other OSes.
This commit mostly just changes the settings code to be in line with
the rest of the platform abstractions, although it also fixes some
settings names to be consistent with others, and uses native bool
types where applicable.
This commit also makes settings-related operations much less
wasteful, not that it should matter.
This commit removes a large amount of code partially duplicated
between the text and the graphics windows, and opens the path to
having more than one model window on screen at any given time,
as well as simplifies platform work.
This commit also adds complete support for High-DPI device pixel
ratio. It adds support for font scale factor (a fractional factor
on top of integral device pixel ratio) on the platform side, but not
on the application side.
This commit also adds error checking to all Windows API calls
(within the abstracted code) and fixes a significant number of
misuses and non-future-proof uses of Windows API.
This commit also makes uses of Windows API idiomatic, e.g. using
the built-in vertical scroll bar, native tooltips, control
subclassing instead of hooks in the global dispatch loop, and so on.
It reinstates tooltip support and removes menu-related hacks.
This commit removes a large amount of redundant code that needed
to be kept in sync between platforms and also makes it much easier
to add new menu-related functionality since little to no platform
code needs to be altered anymore.
This commit also greatly improves code locality in context menu
handling by allowing context menu click handlers to be closures.
This commit temporarily introduces a SetMainMenu API, which is rather
hacky but only necessary until an abstraction for windows is added.
This changes the assertion failure behavior to be the same in debug
and release builds: to show the complete failure message, and
to offer to restart the application or defer to Windows Error
Reporting to generate a backtrace. Contrary to popular belief,
WER is not useless, and since SolveSpace publishes pdb files,
WER-generated reports can be symbolized.
This commit also addresses the long-standing problem where showing
a dialog on fatal error would re-enter the application code, thus
causing another error or a crash that is more fatal than the current
one.
glibc defines a CHAR_WIDTH macro in limits.h since about 6.3.*.
This is apparently added as a part of ISO TS 18661-1:2014, which
I cannot read because it is not publicly available, and which covers
some sort of floating-point extensions. This is one of those changes
that should never have been done yet here we are.
This commit updates a *lot* of rather questionable path handling
logic to be robust. Specifically:
* All path operations go through Platform::Path.
* All ad-hoc path handling functions are removed, together with
PATH_SEP. This removes code that was in platform-independent
parts, but had platform-dependent behavior.
* Group::linkFileRel is removed; only an absolute path is stored
in Group::linkFile. However, only Group::linkFileRel is saved,
with the relative path calculated on the fly, from the filename
passed into SaveToFile. This eliminates dependence on global
state, and makes it unnecessary to have separare code paths
for saved and not yet saved files.
* In a departure from previous practice, functions with
platform-independent code but platform-dependent behavior
are all grouped under platform/. This makes it easy to grep
for functions with platform-dependent behavior.
* Similarly, new (GUI-independent) code for all platforms is added
in the same platform.cpp file, guarded with #ifs. It turns out
that implementations for different platforms had a lot of shared
code that tended to go out of sync.
Before this commit, DoLater would be run as an idle callback,
which (depending on system performance) could either result in
a half-regenerated sketch being displayed, with only the dragged
entity updated, or no regeneration whatsoever during the drag.
After this commit, the GTK behavior matches macOS and Win32 ones.
The somewhat confusingly named set_has_alpha() function does not
affect whether alpha can be used during rendering to the area.
Rather, it affects whether alpha will be used when composing
the contents of the area with the window underneath it.
We want to suppress accelerators but still get input to (at least)
the window where the editor is opened. It's no harm to permit input
to other windows, but it is bad to route all of it to the editor,
since color chooser depends on being able to receive input.
So, what we do is add modal grab to the *overlay*, which has
the editor and the underlay widget, route all events as usual
to children, and just force the key events to go to the editor,
since otherwise they would still propagate up for some reason.
whitequark
ruevs
Koen Schmeets
Thomas Roughton
Ryan A. Pavlik
nabijaczleweli
Ruevski
Zhuowei Zhang
probonopd
Sergiusz Bazanski
Alexander Meißner
luzpaz
EvilSpirit