This is another small profiling driven optimization.
Moving the initialization of `const double *c` as part of the definition
also helps with the generated assembler.
This reverts commit 68b1abf77f.
The warnings are valuable and shouldn't be cast aside.
As of 8f509f1, we special case macOS and don't set -fno-sanitize-recover
to allow CI to succeed.
In the future, this could be made stricter again by only suppressing
known bugs, which ideally should also be fixed or reported upstream.
Profiling with MSVC 2019 showed that many of the Vector methods are on
a critical path (not surprising). They are changed to be inline and
unnecessary temporaries are removed.
On the example below generate times decreased from 102s. to 64s.
At the same time the executable size shrank from 5569536 to 5150208 bytes
in release mode (with global optimizations).
This should not stop us from working on optimizing inner loops e.g. https://github.com/solvespace/solvespace/issues/759 .
[Test model](https://github.com/solvespace/solvespace/files/5414683/PrismConeNURBSNormalsTangents300.zip)
Previously sanitizer flags were set unconditionally for
all code, including that of external libraries.
Set them only for targets in src/, tests/ and exposed/.
Unfortunately, the linker equivalent to add_compile_options,
add_link_options, is only available for CMake version >= 3.13.
So add the sanitizer flags manually to each target's linker options.
The implementation may be sub-optimal, since the colour and alpha is
defined for each NURBS surface instead of on group level, but the STEP
export currently does not represent group structure at all and I am not
familiar with the format in order to change this.
Fixes: https://github.com/solvespace/solvespace/issues/452
- Add OpenMP to macOS build
- Use as many cores as possible in CI
- Update travis osx image to xcode12.2
- Ignore .vscode folder
- In `.travis/sign-macos.sh`, only create keychain when `CI` variable is present
- Only run macOS deploy stage when a tag is pushed
`GetSaveFileNameA` `OPENFILENAMEA` does not like UNC ( "\\\\?\\C:\\..." ) file prefixes in `lpstrFile`.
Work around it by not `Expand`-ing parameters passed on the command line too early.
The only user visible change is that "File|Open Recent" will show items as they
were passed instead of expanded to full path for example:
"..\..\NURBSTests\Intersection2.slvs"
Fixes: https://github.com/solvespace/solvespace/issues/622
When a plane coinsides with a seam we need to copy that trim curve. The existing curve belongs to the original shell surfaces and an intersection is otherwise not found. Fixes#540.
Extrusion direction (normal) of arcs and circles were not taken into
account when importing.
- Add method for calculating a quaternion from extrusion direction
according to DXF arbitrary axis algorithm
- Add required workplanes for arcs not on XY origin plane
- Adjust addDimRadial and addDimDiametric to include normal when
creating associated circle request
This fixes issues #499. The --view option changes projUp and
projRight. For --view to affect the camera, the camera needs to be
initialized using these values.
This set of changes covers all triangulation errors in issue 693. A point coincident with a triangle vertex is OK and needed for bridges, but sometimes the middle point has edges cutting through the triangle that make it a non-ear. We fix that and a couple of off-by-one error (that fixes one of the test cases).
This is the last long-running single-threaded part of the boolean code. On one test model this took Regenerate form 27 seconds down to 18s. The critical sections needed a name (into) because that object must not be modified while in use in different places.
VectorFileWriter::Background() is an empty impl, except that it writes
* a rectangle the size of the output for EPS and PDF, and
* a <style> element setting background-color for SVG
Ref: #525
The 3-plane intersection code fails to converge when a curve joins two tangent NURBS patches. This adds a new function for intersecting exact curves with a surface to avoid those failures. Fixes simplified test model for issue #315.
This allows the selection to reach through entities to their corresponding ones on the underlying sketch which can be dragged if they are not fully constrained. This is decided in a new function Entity::CanBeDragged().
Create a new copy type for faces that includes the translation aspect of helical extrusions. Also swap the end remappings when the shell is inside out - this was also affecting some Revolve extrusions.
* Add a link to its request, unless it's the first entity in that
request (which would just select the same entity again).
* Add a link to its group.
* Add a link to its workplane.
* Add a link to its style; and hide the style row for entities that
aren't stylable.
* Show constraints and measurements (reference constraints) in
separate lists.
* For curve entities, show constraints that apply to the points
related to the curve, not just to the curve itself.
* Show the type of a constraint.
* Show the workplane a constraint is in, for constraints which can
be both projected to workplane or be free in 3d space.
* Clearly distinguish reference from non-reference constraints.
* Add a checkbox for toggling the reference option.
* When showing requests a constraint applies to, highlight on hover
the specific entity being constrained, not just the first one.
This has been completely broken since 2018 (commit a93283df), and no
one noticed, so it probably wasn't useful. Instead of fixing it, just
drop the feature and a bunch of odd nonportable code.
Turning a reference dimension into a constraint one can make
the sketch unsolvable, which suggests Edit → Undo, which would undo
either too much or nothing at all.
The mimalloc temporary heap is a thread-local object that uses RAII
to manage heap lifetimes even in threads that are created implicitly,
e.g. by OpenMP. However, not all threads are necessarily created by
the application; graphics drivers may create their own threads, and
this can lead to deadlocks when combined with library unloading.
Fixes#657.
The heaps are wrapped in a RAIIish thread_local handler,
since being affined affined to a single thread for allocations is
required by the API
Ref: #642
Resolve issue #489 helix has stairsteps.
Force helix axis line to 8 segments.
Grid triangulation to use a minimum of 4 segments for degree>1.
Adds twist dependence for grid triangulation with degree=1.
Added a max_dt parameter for PWL creation and use that for helical edges.
The counter was added solely as a debug feature in commit e7c8c1c8,
which introduced the new Canvas system. It doesn't work all that well
and brings little value, so let's get rid of the visual noise.
This commit continues the work started in commits 521473ee and
e84fd464 that parallelizes certain geometric operations. This commit
cleans up the temporary arena implementations and makes them
thread-safe.
Also, in commit 521473ee, a call to FreeAllTemporary() was added
during initialization to create the heap on Windows. This is now
not necessary as the heap is created transparently on the first call
to AllocTemporary().
When checking the dot product of the tangents `tu` and `tv` to decide
in which direction to split a quad compare it to to LENGTH_EPS instead
of zero to avoid alternating triangle "orientations" when the tangents
are orthogonal (revolve, lathe etc.).
This improves the quality of the resulting triangle mesh.
Commit ea6db67 added an unusual isnan macro:
#define isnan(x) (((x) != (x)) || (x > 1e11) || (x < -1e11))
Commit 8bc322e adds a preprocessor guard that looks like it would
cause the isnan function from math.h to be preferred, but doesn't
actually do that on many platforms, e.g. glibc:
#ifndef isnan
# define isnan(x) (((x) != (x)) || (x > 1e11) || (x < -1e11))
#endif
This commit renames our isnan() to make it clear that it differs
from the standard library operation, and makes it a function.
Fixes#603.
We use std::fabs, but <math.h> doesn't provide it; it worked by accident.
Include <cmath> that provides std::fabs, and switch all other headers to
their C++ variants, too.
* Don't use a reserved identifier in include guards.
* Use fabs() from <cmath> instead of our own ffabs().
This shouldn't make any difference with modern toolchains.
* Convert a few preprocessor macros to constexprs.
After this commit, dbp() is renamed to DebugPrint() and moved to
platform.cpp, next to other similar functions. The existing short
name is provided by a preprocessor macro, similar to ssassert().
This leaves just the (rather hacky) temporary heap in util*.cpp.
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.
This changes all the lambdas to have explicit captures, since the use of
implicit captures has led to some memory errors, especially segfaults in
the right-click menu.
I'm not 100% sure that the code is correct anyway - it really needs auditing
to ensure all referenced values are still valid when the menu item is clicked
(e.g. can you change stuff with keyboard shortcuts while the context menu is
visible?), but it should at least be *more* correct!
This was incorrectly capturing `r` by reference and using it after it left
its scope. Changed to capture by value, and also explicitly capture `this`
in case we were accidentally capturing any other scope variables by reference.
Fixes#571
* Limit u,v range between 0 and 1 in Newton. Fixes issue #471
* Change the math for projecting a point onto a plane to work better with non-orthogonal U,V derivatives in several places. Fixes#472.
See:
https://github.com/solvespace/solvespace/issues/92#issuecomment-567831112
The problem was first introduced here:
dabd57847e
and later "fixed" here:
f324477dd0
by setting the stack size to /STACK:33554432
Solvespace now starts up even with /STACK:554432
According to this:
https://en.cppreference.com/w/cpp/language/value_initialization
```
"2) if T is a class type with a default constructor that is neither user-provided nor deleted (that is, it may be a class with an implicitly-defined or defaulted default constructor), the object is zero-initialized and then it is default-initialized if it has a non-trivial default constructor; "
```
So removing the `{}` should leave both the `System` and `TextWindow` class instances properly initialized.
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'
GetIdent is called from an UI event callback, at which point there
might well not be an active GL context. Before this commit, that
would return a NULL pointer and result in a crash.
Before this commit, certain fonts (e.g. Terminus) would appear in
the selector but cause a crash (assertion failure) if they are used.
After this commit, we make sure all preconditions are met before
showing a font there.
Also, improve error reporting to always print font filename.
This was originally changed in 74aa80b6, but the fix broke stipping
because it incorrectly changed the logic. Revert that, and just make
the textures smaller instead.
Before this commit, resizing the property browser would cut off
the rows at the bottom, or else add black space, until next refresh.
This could be perhaps more elegantly done by adding an onResize event
but given that each of them would be followed by onRender anyway, it
seems there's no benefit to adding onResize.
As I understand it, both glGetError() and glFinish() are serializing
and blockig, so it makes more sense to call them at the same time.
glFlush() does not block.
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.
Union and difference are optimized by replacing the expression
(!inShell && !inFace)
which is equivqlent to
(!inShell && !inSame && !inOpp)
with
outSide
which is equivalent, since SShell::Class::OUTSIDE is the only remaining possibility.
Per the OpenGL documentation:
> GL_INVALID_VALUE may be generated if level is greater than
> log2(max), where max is the returned value of GL_MAX_TEXTURE_SIZE.
Although we always passed `log2(max) + 1` as `level`, for some reason
none of the GL implementations we run on ever returned an error.
It also appears there is a bug in ANGLE that crashes the process
instead in this case if the C++ runtime performs bound checks on
vector::operator[]=.
Was getting segfaults near here with another patch since removed from the branch.
Moving these assignments after the memfree means they still have
useful data when debugging a crash in memfree.
Allows distancing users from the internal "elem" member.
Add Get() and operator[].
Replace direct references to elem.
Make elem and elemsAllocated private in IdList/List.
phkahler
ruevs
Koen Schmeets
Maximilian Federle
julien581
Johannes Rehnman
Christoph Dittmann
nabijaczleweli
whitequark
наб
Ryan Pavlik
Tim
Roc
Thomas Roughton
EvilSpirit
Ryan Pavlik