From 74aa80b645ead723c6f50879ad3bc0c2dc5ee560 Mon Sep 17 00:00:00 2001 From: whitequark Date: Fri, 22 Nov 2019 02:11:00 +0000 Subject: [PATCH 01/24] Fix misuse of glTexImage2D. 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[]=. --- src/render/gl3shader.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/render/gl3shader.cpp b/src/render/gl3shader.cpp index 8ad45c7a..542eb97f 100644 --- a/src/render/gl3shader.cpp +++ b/src/render/gl3shader.cpp @@ -390,7 +390,7 @@ GLuint Generate(const std::vector &pattern) { glGetIntegerv(GL_MAX_TEXTURE_SIZE, &size); RgbaColor *textureData = new RgbaColor[size]; - int mipCount = (int)log2(size) + 1; + int mipCount = (int)log2(size); for(int mip = 0; mip < mipCount; mip++) { int dashI = 0; double dashT = 0.0; From 0dcc8f3369f5d5501fd2fe7eb11cd6514129d195 Mon Sep 17 00:00:00 2001 From: Koen Schmeets Date: Sat, 23 Nov 2019 13:38:41 +0100 Subject: [PATCH 02/24] Fix Xcode auto-fixable issues --- src/platform/guimac.mm | 46 +++++++++++++++++++++--------------------- src/solvespace.cpp | 1 - 2 files changed, 23 insertions(+), 24 deletions(-) diff --git a/src/platform/guimac.mm b/src/platform/guimac.mm index 77f2d565..3d9108b4 100644 --- a/src/platform/guimac.mm +++ b/src/platform/guimac.mm @@ -225,9 +225,9 @@ public: NSUInteger modifierMask = 0; if(accel.shiftDown) - modifierMask |= NSShiftKeyMask; + modifierMask |= NSEventModifierFlagShift; if(accel.controlDown) - modifierMask |= NSCommandKeyMask; + modifierMask |= NSEventModifierFlagCommand; nsMenuItem.keyEquivalentModifierMask = modifierMask; } @@ -236,7 +236,7 @@ public: } void SetActive(bool active) override { - nsMenuItem.state = active ? NSOnState : NSOffState; + nsMenuItem.state = active ? NSControlStateValueOn : NSControlStateValueOff; } void SetEnabled(bool enabled) override { @@ -422,7 +422,7 @@ MenuBarRef GetOrCreateMainMenu(bool *unique) { width * 4, colorspace, kCGBitmapByteOrder32Little | kCGImageAlphaNoneSkipFirst, provider, NULL, true, kCGRenderingIntentDefault); - CGContextDrawImage((CGContextRef) [[NSGraphicsContext currentContext] graphicsPort], + CGContextDrawImage((CGContextRef) [[NSGraphicsContext currentContext] CGContext], [self bounds], image); CGImageRelease(image); @@ -453,8 +453,8 @@ MenuBarRef GetOrCreateMainMenu(bool *unique) { event.y = self.bounds.size.height - nsPoint.y; NSUInteger nsFlags = [nsEvent modifierFlags]; - if(nsFlags & NSShiftKeyMask) event.shiftDown = true; - if(nsFlags & NSCommandKeyMask) event.controlDown = true; + if(nsFlags & NSEventModifierFlagShift) event.shiftDown = true; + if(nsFlags & NSEventModifierFlagCommand) event.controlDown = true; return event; } @@ -587,9 +587,9 @@ MenuBarRef GetOrCreateMainMenu(bool *unique) { KeyboardEvent event = {}; NSUInteger nsFlags = [nsEvent modifierFlags]; - if(nsFlags & NSShiftKeyMask) + if(nsFlags & NSEventModifierFlagShift) event.shiftDown = true; - if(nsFlags & NSCommandKeyMask) + if(nsFlags & NSEventModifierFlagCommand) event.controlDown = true; unichar chr = 0; @@ -610,7 +610,7 @@ MenuBarRef GetOrCreateMainMenu(bool *unique) { - (void)keyDown:(NSEvent *)nsEvent { using Platform::KeyboardEvent; - if([NSEvent modifierFlags] & ~(NSShiftKeyMask|NSCommandKeyMask)) { + if([NSEvent modifierFlags] & ~(NSEventModifierFlagShift|NSEventModifierFlagCommand)) { [super keyDown:nsEvent]; return; } @@ -629,7 +629,7 @@ MenuBarRef GetOrCreateMainMenu(bool *unique) { - (void)keyUp:(NSEvent *)nsEvent { using Platform::KeyboardEvent; - if([NSEvent modifierFlags] & ~(NSShiftKeyMask|NSCommandKeyMask)) { + if([NSEvent modifierFlags] & ~(NSEventModifierFlagShift|NSEventModifierFlagCommand)) { [super keyUp:nsEvent]; return; } @@ -811,16 +811,16 @@ public: switch(kind) { case Window::Kind::TOPLEVEL: nsWindow = [[NSWindow alloc] init]; - nsWindow.styleMask = NSTitledWindowMask | NSResizableWindowMask | - NSClosableWindowMask | NSMiniaturizableWindowMask; + nsWindow.styleMask = NSWindowStyleMaskTitled | NSWindowStyleMaskResizable | + NSWindowStyleMaskClosable | NSWindowStyleMaskMiniaturizable; nsWindow.collectionBehavior = NSWindowCollectionBehaviorFullScreenPrimary; ssView.acceptsFirstResponder = YES; break; case Window::Kind::TOOL: NSPanel *nsPanel = [[NSPanel alloc] init]; - nsPanel.styleMask = NSTitledWindowMask | NSResizableWindowMask | - NSClosableWindowMask | NSUtilityWindowMask; + nsPanel.styleMask = NSWindowStyleMaskTitled | NSWindowStyleMaskResizable | + NSWindowStyleMaskClosable | NSWindowStyleMaskUtilityWindow; [nsPanel standardWindowButton:NSWindowMiniaturizeButton].hidden = YES; [nsPanel standardWindowButton:NSWindowZoomButton].hidden = YES; nsPanel.floatingPanel = YES; @@ -1128,17 +1128,17 @@ void Open3DConnexion() { connexionClient = registerConnexionClient(connexionSignature, connexionName, kConnexionClientModeTakeOver, kConnexionMaskButtons | kConnexionMaskAxis); - [NSEvent addLocalMonitorForEventsMatchingMask:(NSKeyDownMask | NSFlagsChangedMask) + [NSEvent addLocalMonitorForEventsMatchingMask:(NSEventMaskKeyDown | NSEventMaskFlagsChanged) handler:^(NSEvent *event) { - connexionShiftIsDown = (event.modifierFlags & NSShiftKeyMask); - connexionCommandIsDown = (event.modifierFlags & NSCommandKeyMask); + connexionShiftIsDown = (event.modifierFlags & NSEventModifierFlagShift); + connexionCommandIsDown = (event.modifierFlags & NSEventModifierFlagCommand); return event; }]; - [NSEvent addLocalMonitorForEventsMatchingMask:(NSKeyUpMask | NSFlagsChangedMask) + [NSEvent addLocalMonitorForEventsMatchingMask:(NSEventMaskKeyUp | NSEventMaskFlagsChanged) handler:^(NSEvent *event) { - connexionShiftIsDown = (event.modifierFlags & NSShiftKeyMask); - connexionCommandIsDown = (event.modifierFlags & NSCommandKeyMask); + connexionShiftIsDown = (event.modifierFlags & NSEventModifierFlagShift); + connexionCommandIsDown = (event.modifierFlags & NSEventModifierFlagCommand); return event; }]; } @@ -1173,12 +1173,12 @@ public: switch(type) { case Type::INFORMATION: case Type::QUESTION: - nsAlert.alertStyle = NSInformationalAlertStyle; + nsAlert.alertStyle = NSAlertStyleInformational; break; case Type::WARNING: case Type::ERROR: - nsAlert.alertStyle = NSWarningAlertStyle; + nsAlert.alertStyle = NSAlertStyleWarning; break; } } @@ -1289,7 +1289,7 @@ public: } bool RunModal() override { - if([nsPanel runModal] == NSFileHandlingPanelOKButton) { + if([nsPanel runModal] == NSModalResponseOK) { return true; } else { return false; diff --git a/src/solvespace.cpp b/src/solvespace.cpp index 2c0157d0..367ce09e 100644 --- a/src/solvespace.cpp +++ b/src/solvespace.cpp @@ -799,7 +799,6 @@ void SolveSpaceUI::MenuAnalyze(Command id) { Group *g = SK.GetGroup(SS.GW.activeGroup); SS.GW.GroupSelection(); auto const &gs = SS.GW.gs; - double scale = SS.MmPerUnit(); if(gs.faces > 0) { std::vector faces; From b5ccf5acf5f17d7d4ad2929eacbf2080f6082ac3 Mon Sep 17 00:00:00 2001 From: Koen Schmeets Date: Sat, 23 Nov 2019 14:20:45 +0100 Subject: [PATCH 03/24] macOS: fix CMake Xcode generator. This is done by setting `CMAKE_RUNTIME_OUTPUT_DIRECTORY` to `$<1:${CMAKE_BINARY_DIR}/bin>` --- CMakeLists.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index 2f9f7388..e8e5b9b1 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -49,6 +49,9 @@ set(OPENGL 3 CACHE STRING "OpenGL version to use (one of: 1 3)") set(EXECUTABLE_OUTPUT_PATH ${CMAKE_BINARY_DIR}/bin) set(LIBRARY_OUTPUT_PATH ${CMAKE_BINARY_DIR}/bin) +if("${CMAKE_GENERATOR}" STREQUAL "Xcode") + set(CMAKE_RUNTIME_OUTPUT_DIRECTORY $<1:${CMAKE_BINARY_DIR}/bin>) +endif() if(NOT CMAKE_C_COMPILER_ID STREQUAL CMAKE_CXX_COMPILER_ID) message(FATAL_ERROR "C and C++ compilers should be supplied by the same vendor") From ec3056773e2b84803d2acf3b47d653a0fc8cfc46 Mon Sep 17 00:00:00 2001 From: ruevs Date: Sat, 23 Nov 2019 15:22:15 +0200 Subject: [PATCH 04/24] Simplify UNION and DIFFERENCE boolean operations. 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. --- src/srf/boolean.cpp | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src/srf/boolean.cpp b/src/srf/boolean.cpp index 487ed755..f01ce625 100644 --- a/src/srf/boolean.cpp +++ b/src/srf/boolean.cpp @@ -193,28 +193,24 @@ void SSurface::TrimFromEdgeList(SEdgeList *el, bool asUv) { static bool KeepRegion(SSurface::CombineAs type, bool opA, SShell::Class shell, SShell::Class orig) { bool inShell = (shell == SShell::Class::INSIDE), + outSide = (shell == SShell::Class::OUTSIDE), inSame = (shell == SShell::Class::COINC_SAME), - inOpp = (shell == SShell::Class::COINC_OPP), inOrig = (orig == SShell::Class::INSIDE); - bool inFace = inSame || inOpp; - - // If these are correct, then they should be independent of inShell - // if inFace is true. if(!inOrig) return false; switch(type) { case SSurface::CombineAs::UNION: if(opA) { - return (!inShell && !inFace); + return outSide; } else { - return (!inShell && !inFace) || inSame; + return outSide || inSame; } case SSurface::CombineAs::DIFFERENCE: if(opA) { - return (!inShell && !inFace); + return outSide; } else { - return (inShell && !inFace) || inSame; + return inShell || inSame; } default: ssassert(false, "Unexpected combine type"); From 65d0bdffdbd45e8322d759ed7bef1d196ab99b96 Mon Sep 17 00:00:00 2001 From: whitequark Date: Sat, 23 Nov 2019 13:35:12 +0000 Subject: [PATCH 05/24] Split Canvas::FinishFrame out of Canvas::FlushFrame. 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. --- src/draw.cpp | 3 ++- src/platform/entrycli.cpp | 3 ++- src/render/render.h | 6 ++++-- src/render/rendergl1.cpp | 10 ++++++++-- src/render/rendergl3.cpp | 11 ++++++++--- src/textwin.cpp | 3 ++- test/harness.cpp | 3 ++- 7 files changed, 28 insertions(+), 11 deletions(-) diff --git a/src/draw.cpp b/src/draw.cpp index ee3966b2..907ca004 100644 --- a/src/draw.cpp +++ b/src/draw.cpp @@ -845,7 +845,7 @@ void GraphicsWindow::Paint() { canvas->SetLighting(lighting); canvas->SetCamera(camera); - canvas->NewFrame(); + canvas->StartFrame(); Draw(canvas.get()); canvas->FlushFrame(); @@ -902,6 +902,7 @@ void GraphicsWindow::Paint() { 5, 5, renderTimeColor); canvas->FlushFrame(); + canvas->FinishFrame(); canvas->Clear(); } diff --git a/src/platform/entrycli.cpp b/src/platform/entrycli.cpp index b6e1cc01..c9efbf1a 100644 --- a/src/platform/entrycli.cpp +++ b/src/platform/entrycli.cpp @@ -208,9 +208,10 @@ static bool RunCommand(const std::vector args) { pixmapCanvas.SetCamera(camera); pixmapCanvas.Init(); - pixmapCanvas.NewFrame(); + pixmapCanvas.StartFrame(); SS.GW.Draw(&pixmapCanvas); pixmapCanvas.FlushFrame(); + pixmapCanvas.FinishFrame(); pixmapCanvas.ReadFrame()->WritePng(output, /*flip=*/true); pixmapCanvas.Clear(); diff --git a/src/render/render.h b/src/render/render.h index 5c77ebf5..825ca8d1 100644 --- a/src/render/render.h +++ b/src/render/render.h @@ -186,8 +186,9 @@ public: virtual void SetCamera(const Camera &camera) = 0; virtual void SetLighting(const Lighting &lighting) = 0; - virtual void NewFrame() = 0; + virtual void StartFrame() = 0; virtual void FlushFrame() = 0; + virtual void FinishFrame() = 0; virtual std::shared_ptr ReadFrame() = 0; virtual void GetIdent(const char **vendor, const char **renderer, const char **version) = 0; @@ -342,8 +343,9 @@ public: void Clear() override; - void NewFrame() override {} + void StartFrame() override {} void FlushFrame() override; + void FinishFrame() override {} std::shared_ptr ReadFrame() override; void GetIdent(const char **vendor, const char **renderer, const char **version) override; diff --git a/src/render/rendergl1.cpp b/src/render/rendergl1.cpp index f0d41309..070b605e 100644 --- a/src/render/rendergl1.cpp +++ b/src/render/rendergl1.cpp @@ -220,8 +220,9 @@ public: void SetCamera(const Camera &camera) override; void SetLighting(const Lighting &lighting) override; - void NewFrame() override; + void StartFrame() override; void FlushFrame() override; + void FinishFrame() override; std::shared_ptr ReadFrame() override; void GetIdent(const char **vendor, const char **renderer, const char **version) override; @@ -751,7 +752,7 @@ void OpenGl1Renderer::UpdateProjection() { glClear(GL_DEPTH_BUFFER_BIT); } -void OpenGl1Renderer::NewFrame() { +void OpenGl1Renderer::StartFrame() { glEnable(GL_NORMALIZE); glBlendFunc(GL_SRC_ALPHA, GL_ONE_MINUS_SRC_ALPHA); @@ -806,6 +807,7 @@ void OpenGl1Renderer::NewFrame() { void OpenGl1Renderer::FlushFrame() { UnSelectPrimitive(); + glFlush(); GLenum error = glGetError(); @@ -814,6 +816,10 @@ void OpenGl1Renderer::FlushFrame() { } } +void OpenGl1Renderer::FinishFrame() { + glFinish(); +} + std::shared_ptr OpenGl1Renderer::ReadFrame() { int width = camera.width * camera.pixelRatio; int height = camera.height * camera.pixelRatio; diff --git a/src/render/rendergl3.cpp b/src/render/rendergl3.cpp index 61e1084d..592a3445 100644 --- a/src/render/rendergl3.cpp +++ b/src/render/rendergl3.cpp @@ -134,8 +134,9 @@ public: void SetCamera(const Camera &c) override; void SetLighting(const Lighting &l) override; - void NewFrame() override; + void StartFrame() override; void FlushFrame() override; + void FinishFrame() override; void Clear() override; std::shared_ptr ReadFrame() override; @@ -618,7 +619,7 @@ void OpenGl3Renderer::UpdateProjection() { glClear(GL_DEPTH_BUFFER_BIT); } -void OpenGl3Renderer::NewFrame() { +void OpenGl3Renderer::StartFrame() { if(!initialized) { Init(); initialized = true; @@ -667,7 +668,7 @@ void OpenGl3Renderer::FlushFrame() { } points.Clear(); - glFinish(); + glFlush(); GLenum error = glGetError(); if(error != GL_NO_ERROR) { @@ -675,6 +676,10 @@ void OpenGl3Renderer::FlushFrame() { } } +void OpenGl3Renderer::FinishFrame() { + glFinish(); +} + void OpenGl3Renderer::Clear() { ViewportCanvas::Clear(); pixmapCache.CleanupUnused(); diff --git a/src/textwin.cpp b/src/textwin.cpp index 1406a3e7..2a7b3ebf 100644 --- a/src/textwin.cpp +++ b/src/textwin.cpp @@ -924,7 +924,7 @@ void TextWindow::Paint() { canvas->SetLighting(lighting); canvas->SetCamera(camera); - canvas->NewFrame(); + canvas->StartFrame(); UiCanvas uiCanvas = {}; uiCanvas.canvas = canvas; @@ -1041,6 +1041,7 @@ void TextWindow::Paint() { DrawOrHitTestColorPicker(&uiCanvas, PAINT, false, 0, 0); canvas->FlushFrame(); + canvas->FinishFrame(); canvas->Clear(); } diff --git a/test/harness.cpp b/test/harness.cpp index 1927ca2a..60124258 100644 --- a/test/harness.cpp +++ b/test/harness.cpp @@ -255,9 +255,10 @@ bool Test::Helper::CheckRender(const char *file, int line, const char *reference pixmapCanvas.SetCamera(camera); pixmapCanvas.Init(); - pixmapCanvas.NewFrame(); + pixmapCanvas.StartFrame(); SS.GW.Draw(&pixmapCanvas); pixmapCanvas.FlushFrame(); + pixmapCanvas.FinishFrame(); std::shared_ptr frame = pixmapCanvas.ReadFrame(); pixmapCanvas.Clear(); From e386d405d673e93a0448248da415de7f8a5074f0 Mon Sep 17 00:00:00 2001 From: "Ryan A. Pavlik" Date: Sat, 23 Nov 2019 08:07:31 -0600 Subject: [PATCH 06/24] Make some arguments const/const references, where possible. NFC. Found and suggested by clang-tidy.May help performance by reducing copies, depending on frequency of call, etc. --- src/importdxf.cpp | 5 +++-- src/lib.cpp | 2 +- src/platform/gui.h | 2 +- src/platform/guigtk.cpp | 4 ++-- src/platform/guimac.mm | 2 +- src/platform/guinone.cpp | 2 +- src/platform/guiwin.cpp | 5 ++--- src/polygon.h | 9 ++++----- src/polyline.cpp | 7 +++---- src/render/gl3shader.cpp | 14 +++++++------- src/render/gl3shader.h | 14 +++++++------- src/render/render.cpp | 3 +-- src/render/render.h | 2 +- src/resource.cpp | 4 ++-- src/resource.h | 4 ++-- 15 files changed, 38 insertions(+), 41 deletions(-) diff --git a/src/importdxf.cpp b/src/importdxf.cpp index 2db8836f..f5c44086 100644 --- a/src/importdxf.cpp +++ b/src/importdxf.cpp @@ -1068,8 +1068,9 @@ public: } }; -static void ImportDwgDxf(const Platform::Path &filename, - std::function read) { +static void +ImportDwgDxf(const Platform::Path &filename, + const std::function &read) { std::string fileType = ToUpper(filename.Extension()); std::string data; diff --git a/src/lib.cpp b/src/lib.cpp index 052768d2..c16206b1 100644 --- a/src/lib.cpp +++ b/src/lib.cpp @@ -13,7 +13,7 @@ static System SYS; static int IsInit = 0; -void SolveSpace::Platform::FatalError(std::string message) { +void SolveSpace::Platform::FatalError(const std::string &message) { fprintf(stderr, "%s", message.c_str()); abort(); } diff --git a/src/platform/gui.h b/src/platform/gui.h index 50bebc3e..c5d082c1 100644 --- a/src/platform/gui.h +++ b/src/platform/gui.h @@ -102,7 +102,7 @@ std::string AcceleratorDescription(const KeyboardEvent &accel); #if defined(__GNUC__) __attribute__((noreturn)) #endif -void FatalError(std::string message); +void FatalError(const std::string &message); // A native settings store. class Settings { diff --git a/src/platform/guigtk.cpp b/src/platform/guigtk.cpp index 61181482..73fb01ec 100644 --- a/src/platform/guigtk.cpp +++ b/src/platform/guigtk.cpp @@ -50,7 +50,7 @@ static std::string PrepareMnemonics(std::string label) { return label; } -static std::string PrepareTitle(std::string title) { +static std::string PrepareTitle(const std::string &title) { return title + " — SolveSpace"; } @@ -58,7 +58,7 @@ static std::string PrepareTitle(std::string title) { // Fatal errors //----------------------------------------------------------------------------- -void FatalError(std::string message) { +void FatalError(const std::string &message) { fprintf(stderr, "%s", message.c_str()); abort(); } diff --git a/src/platform/guimac.mm b/src/platform/guimac.mm index 3d9108b4..0a1078df 100644 --- a/src/platform/guimac.mm +++ b/src/platform/guimac.mm @@ -84,7 +84,7 @@ crash_info_t crashAnnotation __attribute__((section("__DATA,__crash_info"))) = { CRASH_VERSION, NULL, NULL, NULL, NULL, NULL, NULL }; -void FatalError(std::string message) { +void FatalError(const std::string &message) { crashAnnotation.message = message.c_str(); abort(); } diff --git a/src/platform/guinone.cpp b/src/platform/guinone.cpp index 31f37dcd..85beae37 100644 --- a/src/platform/guinone.cpp +++ b/src/platform/guinone.cpp @@ -22,7 +22,7 @@ namespace Platform { // Fatal errors //----------------------------------------------------------------------------- -void FatalError(std::string message) { +void FatalError(const std::string &message) { fprintf(stderr, "%s", message.c_str()); #if !defined(LIBRARY) && defined(HAVE_BACKTRACE) diff --git a/src/platform/guiwin.cpp b/src/platform/guiwin.cpp index 40f20c51..b2ae8105 100644 --- a/src/platform/guiwin.cpp +++ b/src/platform/guiwin.cpp @@ -152,13 +152,12 @@ static int Clamp(int x, int a, int b) { bool handlingFatalError = false; -void FatalError(std::string message) { +void FatalError(const std::string &message) { // Indicate that we're handling a fatal error, to avoid re-entering application code // and potentially crashing even harder. handlingFatalError = true; - message += "\nGenerate debug report?"; - switch(MessageBoxW(NULL, Platform::Widen(message).c_str(), + switch(MessageBoxW(NULL, Platform::Widen(message + "\nGenerate debug report?").c_str(), L"Fatal error — SolveSpace", MB_ICONERROR|MB_TASKMODAL|MB_SETFOREGROUND|MB_TOPMOST| MB_OKCANCEL|MB_DEFBUTTON2)) { diff --git a/src/polygon.h b/src/polygon.h index b7c5a71e..a9a754c4 100644 --- a/src/polygon.h +++ b/src/polygon.h @@ -403,11 +403,10 @@ public: Vertex *AddVertex(const Vector &pos); Edge *AddEdge(const Vector &p0, const Vector &p1, uint32_t kind, uintptr_t data = 0); - void Generate( - std::function startFunc, - std::function nextFunc, - std::function aloneFunc, - std::function endFunc = [](){}); + void Generate(std::function const &startFunc, + std::function const &nextFunc, + std::function const &aloneFunc, + std::function const &endFunc = []() {}); void MakeFromEdges(const SEdgeList &sel); void MakeFromOutlines(const SOutlineList &sol); diff --git a/src/polyline.cpp b/src/polyline.cpp index bddf1ab7..64aef4e9 100644 --- a/src/polyline.cpp +++ b/src/polyline.cpp @@ -141,10 +141,9 @@ PolylineBuilder::Edge *PolylineBuilder::AddEdge(const Vector &p0, const Vector & } void PolylineBuilder::Generate( - std::function startFunc, - std::function nextFunc, - std::function aloneFunc, - std::function endFunc) { + std::function const &startFunc, + std::function const &nextFunc, + std::function const &aloneFunc, std::function const &endFunc) { bool found; bool loop = false; do { diff --git a/src/render/gl3shader.cpp b/src/render/gl3shader.cpp index 542eb97f..888779d3 100644 --- a/src/render/gl3shader.cpp +++ b/src/render/gl3shader.cpp @@ -162,7 +162,7 @@ void Shader::Clear() { glDeleteProgram(program); } -void Shader::SetUniformMatrix(const char *name, double *md) { +void Shader::SetUniformMatrix(const char *name, const double *md) { Enable(); float mf[16]; for(int i = 0; i < 16; i++) mf[i] = (float)md[i]; @@ -621,11 +621,11 @@ void EdgeRenderer::Draw(const SEdgeList &edges) { Remove(handle); } -void EdgeRenderer::SetModelview(double *matrix) { +void EdgeRenderer::SetModelview(const double *matrix) { shader.SetUniformMatrix("modelview", matrix); } -void EdgeRenderer::SetProjection(double *matrix) { +void EdgeRenderer::SetProjection(const double *matrix) { shader.SetUniformMatrix("projection", matrix); } @@ -833,11 +833,11 @@ void OutlineRenderer::Draw(const SOutlineList &outlines, Canvas::DrawOutlinesAs Remove(handle); } -void OutlineRenderer::SetModelview(double *matrix) { +void OutlineRenderer::SetModelview(const double *matrix) { shader.SetUniformMatrix("modelview", matrix); } -void OutlineRenderer::SetProjection(double *matrix) { +void OutlineRenderer::SetProjection(const double *matrix) { shader.SetUniformMatrix("projection", matrix); } @@ -1037,14 +1037,14 @@ bool IndexedMeshRenderer::NeedsTexture() const { selectedShader == &pointShader; } -void IndexedMeshRenderer::SetModelview(double *matrix) { +void IndexedMeshRenderer::SetModelview(const double *matrix) { colShader.SetUniformMatrix("modelview", matrix); texShader.SetUniformMatrix("modelview", matrix); texaShader.SetUniformMatrix("modelview", matrix); pointShader.SetUniformMatrix("modelview", matrix); } -void IndexedMeshRenderer::SetProjection(double *matrix) { +void IndexedMeshRenderer::SetProjection(const double *matrix) { colShader.SetUniformMatrix("projection", matrix); texShader.SetUniformMatrix("projection", matrix); texaShader.SetUniformMatrix("projection", matrix); diff --git a/src/render/gl3shader.h b/src/render/gl3shader.h index ae5ed33d..1831e792 100644 --- a/src/render/gl3shader.h +++ b/src/render/gl3shader.h @@ -74,7 +74,7 @@ public: const std::vector> &locations = {}); void Clear(); - void SetUniformMatrix(const char *name, double *md); + void SetUniformMatrix(const char *name, const double *md); void SetUniformVector(const char *name, const Vector &v); void SetUniformVector(const char *name, const Vector4f &v); void SetUniformColor(const char *name, RgbaColor c); @@ -163,8 +163,8 @@ public: void Draw(const Handle &handle); void Draw(const SEdgeList &edges); - void SetModelview(double *matrix); - void SetProjection(double *matrix); + void SetModelview(const double *matrix); + void SetProjection(const double *matrix); void SetStroke(const Canvas::Stroke &stroke, double pixel); }; @@ -203,8 +203,8 @@ public: void Draw(const Handle &handle, Canvas::DrawOutlinesAs mode); void Draw(const SOutlineList &outlines, Canvas::DrawOutlinesAs mode); - void SetModelview(double *matrix); - void SetProjection(double *matrix); + void SetModelview(const double *matrix); + void SetProjection(const double *matrix); void SetStroke(const Canvas::Stroke &stroke, double pixel); }; @@ -252,8 +252,8 @@ public: void Draw(const Handle &handle); void Draw(const SIndexedMesh &mesh); - void SetModelview(double *matrix); - void SetProjection(double *matrix); + void SetModelview(const double *matrix); + void SetProjection(const double *matrix); bool NeedsTexture() const; diff --git a/src/render/render.cpp b/src/render/render.cpp index 3ef7171b..ca2a26eb 100644 --- a/src/render/render.cpp +++ b/src/render/render.cpp @@ -444,12 +444,11 @@ void ObjectPicker::DrawPixmap(std::shared_ptr pm, DrawQuad(o, o.Plus(u), o.Plus(u).Plus(v), o.Plus(v), hcf); } -bool ObjectPicker::Pick(std::function drawFn) { +bool ObjectPicker::Pick(const std::function &drawFn) { minDistance = VERY_POSITIVE; maxZIndex = INT_MIN; drawFn(); return minDistance < selRadius; } - } diff --git a/src/render/render.h b/src/render/render.h index 825ca8d1..649256e7 100644 --- a/src/render/render.h +++ b/src/render/render.h @@ -261,7 +261,7 @@ public: void DoQuad(const Vector &a, const Vector &b, const Vector &c, const Vector &d, int zIndex, int comparePosition = 0); - bool Pick(std::function drawFn); + bool Pick(const std::function &drawFn); }; // A canvas that renders onto a 2d surface, performing z-index sorting, occlusion testing, etc, diff --git a/src/resource.cpp b/src/resource.cpp index 6e9ce727..7b19081e 100644 --- a/src/resource.cpp +++ b/src/resource.cpp @@ -918,7 +918,7 @@ Vector VectorFont::GetExtents(double forCapHeight, const std::string &str) { } void VectorFont::Trace(double forCapHeight, Vector o, Vector u, Vector v, const std::string &str, - std::function traceEdge) { + const std::function &traceEdge) { ssassert(!IsEmpty(), "Expected a loaded font"); double scale = (forCapHeight / capHeight); @@ -945,7 +945,7 @@ void VectorFont::Trace(double forCapHeight, Vector o, Vector u, Vector v, const } void VectorFont::Trace(double forCapHeight, Vector o, Vector u, Vector v, const std::string &str, - std::function traceEdge, const Camera &camera) { + const std::function &traceEdge, const Camera &camera) { ssassert(!IsEmpty(), "Expected a loaded font"); // Perform grid-fitting only when the text is parallel to the view plane. diff --git a/src/resource.h b/src/resource.h index 6c14dad8..bb0e1c7a 100644 --- a/src/resource.h +++ b/src/resource.h @@ -103,9 +103,9 @@ public: Vector GetExtents(double forCapHeight, const std::string &str); void Trace(double forCapHeight, Vector o, Vector u, Vector v, const std::string &str, - std::function traceEdge); + const std::function &traceEdge); void Trace(double forCapHeight, Vector o, Vector u, Vector v, const std::string &str, - std::function traceEdge, const Camera &camera); + const std::function &traceEdge, const Camera &camera); }; #endif From 9b61f988be45cc4122425e6d7d7304c027cd8c02 Mon Sep 17 00:00:00 2001 From: Thomas Roughton Date: Sat, 23 Nov 2019 13:29:09 +0100 Subject: [PATCH 07/24] macOS: Don't use offscreen rendering. 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 --- src/platform/guimac.mm | 53 ++++++++++++------------------------------ 1 file changed, 15 insertions(+), 38 deletions(-) diff --git a/src/platform/guimac.mm b/src/platform/guimac.mm index 0a1078df..ad4871a4 100644 --- a/src/platform/guimac.mm +++ b/src/platform/guimac.mm @@ -344,7 +344,7 @@ MenuBarRef GetOrCreateMainMenu(bool *unique) { // Cocoa NSView and NSWindow extensions //----------------------------------------------------------------------------- -@interface SSView : NSView +@interface SSView : NSOpenGLView @property Platform::Window *receiver; @property BOOL acceptsFirstResponder; @@ -362,8 +362,6 @@ MenuBarRef GetOrCreateMainMenu(bool *unique) { @implementation SSView { - GlOffscreen offscreen; - NSOpenGLContext *glContext; NSTrackingArea *trackingArea; NSTextField *editor; } @@ -371,17 +369,15 @@ MenuBarRef GetOrCreateMainMenu(bool *unique) { @synthesize acceptsFirstResponder; - (id)initWithFrame:(NSRect)frameRect { - if(self = [super initWithFrame:frameRect]) { + NSOpenGLPixelFormatAttribute attrs[] = { + NSOpenGLPFAColorSize, 24, + NSOpenGLPFADepthSize, 24, + 0 + }; + NSOpenGLPixelFormat *pixelFormat = [[NSOpenGLPixelFormat alloc] initWithAttributes:attrs]; + if(self = [super initWithFrame:frameRect pixelFormat:pixelFormat]) { + self.wantsBestResolutionOpenGLSurface = YES; self.wantsLayer = YES; - - NSOpenGLPixelFormatAttribute attrs[] = { - NSOpenGLPFAColorSize, 24, - NSOpenGLPFADepthSize, 24, - 0 - }; - NSOpenGLPixelFormat *pixelFormat = [[NSOpenGLPixelFormat alloc] initWithAttributes:attrs]; - glContext = [[NSOpenGLContext alloc] initWithFormat:pixelFormat shareContext:NULL]; - editor = [[NSTextField alloc] init]; editor.editable = YES; [[editor cell] setWraps:NO]; @@ -394,7 +390,6 @@ MenuBarRef GetOrCreateMainMenu(bool *unique) { } - (void)dealloc { - offscreen.Clear(); } - (BOOL)isFlipped { @@ -404,29 +399,11 @@ MenuBarRef GetOrCreateMainMenu(bool *unique) { @synthesize receiver; - (void)drawRect:(NSRect)aRect { - [glContext makeCurrentContext]; - - NSSize size = [self convertSizeToBacking:self.bounds.size]; - int width = (int)size.width, - height = (int)size.height; - offscreen.Render(width, height, [&] { - if(receiver->onRender) { - receiver->onRender(); - } - }); - - CGDataProviderRef provider = CGDataProviderCreateWithData( - NULL, &offscreen.data[0], width * height * 4, NULL); - CGColorSpaceRef colorspace = CGColorSpaceCreateDeviceRGB(); - CGImageRef image = CGImageCreate(width, height, 8, 32, - width * 4, colorspace, kCGBitmapByteOrder32Little | kCGImageAlphaNoneSkipFirst, - provider, NULL, true, kCGRenderingIntentDefault); - - CGContextDrawImage((CGContextRef) [[NSGraphicsContext currentContext] CGContext], - [self bounds], image); - - CGImageRelease(image); - CGDataProviderRelease(provider); + [[self openGLContext] makeCurrentContext]; + if(receiver->onRender) { + receiver->onRender(); + } + [[self openGLContext] flushBuffer]; } - (BOOL)acceptsFirstMouse:(NSEvent *)event { @@ -892,7 +869,7 @@ public: } void GetContentSize(double *width, double *height) override { - NSSize nsSize = [ssView frame].size; + NSSize nsSize = ssView.frame.size; *width = nsSize.width; *height = nsSize.height; } From 54015b6777d2195d54f1738d80c5aa20f194a6b8 Mon Sep 17 00:00:00 2001 From: whitequark Date: Sat, 23 Nov 2019 14:49:41 +0000 Subject: [PATCH 08/24] Call glGetError() after glFinish(), not glFlush(). 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. --- src/render/rendergl1.cpp | 8 ++++---- src/render/rendergl3.cpp | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/render/rendergl1.cpp b/src/render/rendergl1.cpp index 070b605e..3e6feb0b 100644 --- a/src/render/rendergl1.cpp +++ b/src/render/rendergl1.cpp @@ -809,6 +809,10 @@ void OpenGl1Renderer::FlushFrame() { UnSelectPrimitive(); glFlush(); +} + +void OpenGl1Renderer::FinishFrame() { + glFinish(); GLenum error = glGetError(); if(error != GL_NO_ERROR) { @@ -816,10 +820,6 @@ void OpenGl1Renderer::FlushFrame() { } } -void OpenGl1Renderer::FinishFrame() { - glFinish(); -} - std::shared_ptr OpenGl1Renderer::ReadFrame() { int width = camera.width * camera.pixelRatio; int height = camera.height * camera.pixelRatio; diff --git a/src/render/rendergl3.cpp b/src/render/rendergl3.cpp index 592a3445..b3ae1fa6 100644 --- a/src/render/rendergl3.cpp +++ b/src/render/rendergl3.cpp @@ -669,6 +669,10 @@ void OpenGl3Renderer::FlushFrame() { points.Clear(); glFlush(); +} + +void OpenGl3Renderer::FinishFrame() { + glFinish(); GLenum error = glGetError(); if(error != GL_NO_ERROR) { @@ -676,10 +680,6 @@ void OpenGl3Renderer::FlushFrame() { } } -void OpenGl3Renderer::FinishFrame() { - glFinish(); -} - void OpenGl3Renderer::Clear() { ViewportCanvas::Clear(); pixmapCache.CleanupUnused(); From f0359556d8418b247806fab3737dc06c24694f68 Mon Sep 17 00:00:00 2001 From: whitequark Date: Sat, 23 Nov 2019 15:04:31 +0000 Subject: [PATCH 09/24] Recompute property browser viewport on resize. 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. --- src/textwin.cpp | 29 +++++++++++++++++------------ src/ui.h | 1 + 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/src/textwin.cpp b/src/textwin.cpp index 2a7b3ebf..11d30484 100644 --- a/src/textwin.cpp +++ b/src/textwin.cpp @@ -563,21 +563,24 @@ void TextWindow::Show() { } } - if(window) { - double width, height; - window->GetContentSize(&width, &height); + if(window) Resize(); +} - halfRows = (int)height / (LINE_HEIGHT/2); +void TextWindow::Resize() +{ + double width, height; + window->GetContentSize(&width, &height); - int bottom = top[rows-1] + 2; - scrollPos = min(scrollPos, bottom - halfRows); - scrollPos = max(scrollPos, 0); + halfRows = (int)height / (LINE_HEIGHT/2); - window->ConfigureScrollbar(0, top[rows - 1] + 1, halfRows); - window->SetScrollbarPosition(scrollPos); - window->SetScrollbarVisible(top[rows - 1] + 1 > halfRows); - window->Invalidate(); - } + int bottom = top[rows-1] + 2; + scrollPos = min(scrollPos, bottom - halfRows); + scrollPos = max(scrollPos, 0); + + window->ConfigureScrollbar(0, top[rows - 1] + 1, halfRows); + window->SetScrollbarPosition(scrollPos); + window->SetScrollbarVisible(top[rows - 1] + 1 > halfRows); + window->Invalidate(); } void TextWindow::DrawOrHitTestIcons(UiCanvas *uiCanvas, TextWindow::DrawOrHitHow how, @@ -909,6 +912,8 @@ void TextWindow::Paint() { double width, height; window->GetContentSize(&width, &height); + if(halfRows != (int)height / (LINE_HEIGHT/2)) + Resize(); Camera camera = {}; camera.width = width; diff --git a/src/ui.h b/src/ui.h index 284e9d60..a2985598 100644 --- a/src/ui.h +++ b/src/ui.h @@ -253,6 +253,7 @@ public: void ClearScreen(); void Show(); + void Resize(); // State for the screen that we are showing in the text window. enum class Screen : uint32_t { From 8c750cef9cdad542f1539e67bf69ec3f263e3e0f Mon Sep 17 00:00:00 2001 From: whitequark Date: Sat, 23 Nov 2019 15:06:36 +0000 Subject: [PATCH 10/24] Freeze the scrollbar while editor is open in property browser. Before this commit, the scrollbar would move freely, without changing the position of the viewport. It would be reset after editing is finished. --- src/platform/guimac.mm | 5 +++-- src/platform/guiwin.cpp | 6 +++++- src/textwin.cpp | 4 +++- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/platform/guimac.mm b/src/platform/guimac.mm index ad4871a4..33901ff6 100644 --- a/src/platform/guimac.mm +++ b/src/platform/guimac.mm @@ -952,9 +952,10 @@ public: } void SetScrollbarPosition(double pos) override { - if(pos > ssView.scrollerMax) { + if(pos > ssView.scrollerMax) pos = ssView.scrollerMax; - } + if(GetScrollbarPosition() == pos) + return; [nsScroller setDoubleValue:(pos / (ssView.scrollerMax - ssView.scrollerMin))]; if(onScrollbarAdjusted) { onScrollbarAdjusted(pos); diff --git a/src/platform/guiwin.cpp b/src/platform/guiwin.cpp index b2ae8105..bd1b42e7 100644 --- a/src/platform/guiwin.cpp +++ b/src/platform/guiwin.cpp @@ -1354,7 +1354,11 @@ public: SCROLLINFO si = {}; si.cbSize = sizeof(si); si.fMask = SIF_POS; - si.nPos = (UINT)(pos * SCROLLBAR_UNIT); + sscheck(GetScrollInfo(hWindow, SB_VERT, &si)); + if(si.nPos == (int)(pos * SCROLLBAR_UNIT)) + return; + + si.nPos = (int)(pos * SCROLLBAR_UNIT); sscheck(SetScrollInfo(hWindow, SB_VERT, &si, /*redraw=*/TRUE)); // Windows won't synthesize a WM_VSCROLL for us here. diff --git a/src/textwin.cpp b/src/textwin.cpp index 11d30484..131e12e9 100644 --- a/src/textwin.cpp +++ b/src/textwin.cpp @@ -1129,8 +1129,10 @@ void TextWindow::MouseLeave() { } void TextWindow::ScrollbarEvent(double newPos) { - if(window->IsEditorVisible()) + if(window->IsEditorVisible()) { + window->SetScrollbarPosition(scrollPos); return; + } int bottom = top[rows-1] + 2; newPos = min((int)newPos, bottom - halfRows); From 93184c54acf70324dca8c9bce8c51c7ba3b6c0d9 Mon Sep 17 00:00:00 2001 From: Ryan Pavlik Date: Tue, 20 Aug 2019 14:53:54 -0500 Subject: [PATCH 11/24] Use standard std::count_if. NFC. --- src/dsc.h | 5 ----- src/group.cpp | 3 ++- src/polygon.cpp | 27 ++++++++------------------- 3 files changed, 10 insertions(+), 25 deletions(-) diff --git a/src/dsc.h b/src/dsc.h index 087a664f..ce92c596 100644 --- a/src/dsc.h +++ b/src/dsc.h @@ -480,11 +480,6 @@ public: const T *cbegin() const { return begin(); } const T *cend() const { return end(); } - template - size_t CountIf(F &&predicate) const { - return std::count_if(begin(), end(), std::forward(predicate)); - } - void ClearTags() { for(auto &elt : *this) { elt.tag = 0; } } diff --git a/src/group.cpp b/src/group.cpp index 3e863f55..ad00efe1 100644 --- a/src/group.cpp +++ b/src/group.cpp @@ -50,7 +50,8 @@ bool Group::IsVisible() { } size_t Group::GetNumConstraints() { - return SK.constraint.CountIf([&](Constraint const & c) { return c.group == h; }); + return std::count_if(SK.constraint.begin(), SK.constraint.end(), + [&](Constraint const &c) { return c.group == h; }); } Vector Group::ExtrusionGetVector() { diff --git a/src/polygon.cpp b/src/polygon.cpp index 4589f81a..33f9b010 100644 --- a/src/polygon.cpp +++ b/src/polygon.cpp @@ -302,16 +302,10 @@ bool SEdgeList::AssemblePolygon(SPolygon *dest, SEdge *errorAt, bool keepDir) co // but they are considered to cross if they are coincident and overlapping. // If pi is not NULL, then a crossing is returned in that. //----------------------------------------------------------------------------- -int SEdgeList::AnyEdgeCrossings(Vector a, Vector b, - Vector *ppi, SPointList *spl) const -{ - int cnt = 0; - for(const SEdge *se = l.First(); se; se = l.NextAfter(se)) { - if(se->EdgeCrosses(a, b, ppi, spl)) { - cnt++; - } - } - return cnt; +int SEdgeList::AnyEdgeCrossings(Vector a, Vector b, Vector *ppi, SPointList *spl) const { + auto cnt = std::count_if(l.begin(), l.end(), + [&](SEdge const &se) { return se.EdgeCrosses(a, b, ppi, spl); }); + return static_cast(cnt); } //----------------------------------------------------------------------------- @@ -707,15 +701,10 @@ bool SPolygon::ContainsPoint(Vector p) const { } int SPolygon::WindingNumberForPoint(Vector p) const { - int winding = 0; - int i; - for(i = 0; i < l.n; i++) { - const SContour *sc = &(l[i]); - if(sc->ContainsPointProjdToNormal(normal, p)) { - winding++; - } - } - return winding; + auto winding = std::count_if(l.begin(), l.end(), [&](const SContour &sc) { + return sc.ContainsPointProjdToNormal(normal, p); + }); + return static_cast(winding); } void SPolygon::FixContourDirections() { From 38f42871c71ed57477d5f0985b9730abc6bbb97c Mon Sep 17 00:00:00 2001 From: Ryan Pavlik Date: Wed, 11 Sep 2019 13:48:27 -0500 Subject: [PATCH 12/24] Remove an integer cast. --- src/polygon.cpp | 4 ++-- src/polygon.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/polygon.cpp b/src/polygon.cpp index 33f9b010..4adee80a 100644 --- a/src/polygon.cpp +++ b/src/polygon.cpp @@ -700,11 +700,11 @@ bool SPolygon::ContainsPoint(Vector p) const { return (WindingNumberForPoint(p) % 2) == 1; } -int SPolygon::WindingNumberForPoint(Vector p) const { +size_t SPolygon::WindingNumberForPoint(Vector p) const { auto winding = std::count_if(l.begin(), l.end(), [&](const SContour &sc) { return sc.ContainsPointProjdToNormal(normal, p); }); - return static_cast(winding); + return winding; } void SPolygon::FixContourDirections() { diff --git a/src/polygon.h b/src/polygon.h index a9a754c4..29cc0aa9 100644 --- a/src/polygon.h +++ b/src/polygon.h @@ -149,7 +149,7 @@ public: Vector ComputeNormal() const; void AddEmptyContour(); - int WindingNumberForPoint(Vector p) const; + size_t WindingNumberForPoint(Vector p) const; double SignedArea() const; bool ContainsPoint(Vector p) const; void MakeEdgesInto(SEdgeList *el) const; From 2fe17a46c28021d25eb9228e31c2818d8eb140f5 Mon Sep 17 00:00:00 2001 From: Ryan Pavlik Date: Wed, 11 Sep 2019 13:48:47 -0500 Subject: [PATCH 13/24] Remove prototype for an unimplemented function. --- src/polygon.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/polygon.h b/src/polygon.h index 29cc0aa9..a4be64bb 100644 --- a/src/polygon.h +++ b/src/polygon.h @@ -184,7 +184,6 @@ public: Vector Normal() const; void FlipNormal(); double MinAltitude() const; - int WindingNumberForPoint(Vector p) const; bool ContainsPoint(Vector p) const; bool ContainsPointProjd(Vector n, Vector p) const; STriangle Transform(Vector o, Vector u, Vector v) const; From e74137dc6716d5aa58130de58dffc2fc1a130362 Mon Sep 17 00:00:00 2001 From: whitequark Date: Sat, 23 Nov 2019 15:56:57 +0000 Subject: [PATCH 14/24] Fix misuse of glTexImage2D (again). 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. --- src/render/gl3shader.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/render/gl3shader.cpp b/src/render/gl3shader.cpp index 888779d3..a3090468 100644 --- a/src/render/gl3shader.cpp +++ b/src/render/gl3shader.cpp @@ -388,9 +388,10 @@ GLuint Generate(const std::vector &pattern) { GLint size; glGetIntegerv(GL_MAX_TEXTURE_SIZE, &size); - RgbaColor *textureData = new RgbaColor[size]; + size /= 2; - int mipCount = (int)log2(size); + RgbaColor *textureData = new RgbaColor[size]; + int mipCount = (int)log2(size) + 1; for(int mip = 0; mip < mipCount; mip++) { int dashI = 0; double dashT = 0.0; @@ -423,8 +424,8 @@ GLuint Generate(const std::vector &pattern) { textureData); size /= 2; } - delete []textureData; + return texture; } From 14e095c93aeeb1e23959e4615549af47c5cc79e3 Mon Sep 17 00:00:00 2001 From: whitequark Date: Sat, 23 Nov 2019 16:16:26 +0000 Subject: [PATCH 15/24] Don't show unusable fonts in font selector. 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. --- src/ttf.cpp | 64 +++++++++++++++++++++++++++++------------------------ src/ttf.h | 6 ++--- 2 files changed, 38 insertions(+), 32 deletions(-) diff --git a/src/ttf.cpp b/src/ttf.cpp index ebf7cc56..ddb2aa17 100644 --- a/src/ttf.cpp +++ b/src/ttf.cpp @@ -97,9 +97,9 @@ TtfFont *TtfFontList::LoadFont(const std::string &font) if(tf != l.end()) { if(tf->fontFace == NULL) { if(tf->IsResource()) - tf->LoadFromResource(fontLibrary, /*nameOnly=*/false); + tf->LoadFromResource(fontLibrary, /*keepOpen=*/true); else - tf->LoadFromFile(fontLibrary, /*nameOnly=*/false); + tf->LoadFromFile(fontLibrary, /*keepOpen=*/true); } return tf; } else { @@ -155,7 +155,7 @@ bool TtfFont::IsResource() const { //----------------------------------------------------------------------------- // Load a TrueType font into memory. //----------------------------------------------------------------------------- -bool TtfFont::LoadFromFile(FT_Library fontLibrary, bool nameOnly) { +bool TtfFont::LoadFromFile(FT_Library fontLibrary, bool keepOpen) { ssassert(!IsResource(), "Cannot load a font provided by a resource as a file."); FT_Open_Args args = {}; @@ -171,14 +171,14 @@ bool TtfFont::LoadFromFile(FT_Library fontLibrary, bool nameOnly) { return false; } - return ExtractTTFData(nameOnly); + return ExtractTTFData(keepOpen); } //----------------------------------------------------------------------------- // Load a TrueType from resource in memory. Implemented to load bundled fonts // through theresource system. //----------------------------------------------------------------------------- -bool TtfFont::LoadFromResource(FT_Library fontLibrary, bool nameOnly) { +bool TtfFont::LoadFromResource(FT_Library fontLibrary, bool keepOpen) { ssassert(IsResource(), "Font to be loaded as resource is not provided by a resource " "or does not have the 'res://' prefix."); @@ -196,13 +196,13 @@ bool TtfFont::LoadFromResource(FT_Library fontLibrary, bool nameOnly) { return false; } - return ExtractTTFData(nameOnly); + return ExtractTTFData(keepOpen); } //----------------------------------------------------------------------------- // Extract font information. We care about the font name and unit size. //----------------------------------------------------------------------------- -bool TtfFont::ExtractTTFData(bool nameOnly) { +bool TtfFont::ExtractTTFData(bool keepOpen) { if(int fterr = FT_Select_Charmap(fontFace, FT_ENCODING_UNICODE)) { dbp("freetype: loading unicode CMap for file '%s' failed: %s", fontFile.raw.c_str(), ft_error_string(fterr)); @@ -214,12 +214,6 @@ bool TtfFont::ExtractTTFData(bool nameOnly) { name = std::string(fontFace->family_name) + " (" + std::string(fontFace->style_name) + ")"; - if(nameOnly) { - FT_Done_Face(fontFace); - fontFace = NULL; - return true; - } - // We always ask Freetype to give us a unit size character. // It uses fixed point; put the unit size somewhere in the middle of the dynamic // range of its 26.6 fixed point type, and adjust the factors so that the unit @@ -231,8 +225,8 @@ bool TtfFont::ExtractTTFData(bool nameOnly) { sizeRequest.horiResolution = 128; sizeRequest.vertResolution = 128; if(int fterr = FT_Request_Size(fontFace, &sizeRequest)) { - dbp("freetype: cannot set character size: %s", - ft_error_string(fterr)); + dbp("freetype: size request for file '%s' failed: %s", + fontFile.raw.c_str(), ft_error_string(fterr)); FT_Done_Face(fontFace); fontFace = NULL; return false; @@ -241,16 +235,17 @@ bool TtfFont::ExtractTTFData(bool nameOnly) { char chr = 'A'; uint32_t gid = FT_Get_Char_Index(fontFace, 'A'); if (gid == 0) { - dbp("freetype: CID-to-GID mapping for CID 0x%04x failed: %s; using CID as GID", - chr, ft_error_string(gid)); + dbp("freetype: CID-to-GID mapping for CID 0x%04x in file '%s' failed: %s; " + "using CID as GID", + chr, fontFile.raw.c_str(), ft_error_string(gid)); dbp("Assuming cap height is the same as requested height (this is likely wrong)."); capHeight = (double)sizeRequest.height; } if(gid) { if(int fterr = FT_Load_Glyph(fontFace, gid, FT_LOAD_NO_BITMAP | FT_LOAD_NO_HINTING)) { - dbp("freetype: cannot load glyph for GID 0x%04x: %s", - gid, ft_error_string(fterr)); + dbp("freetype: cannot load glyph for GID 0x%04x in file '%s': %s", + gid, fontFile.raw.c_str(), ft_error_string(fterr)); FT_Done_Face(fontFace); fontFace = NULL; return false; @@ -261,6 +256,15 @@ bool TtfFont::ExtractTTFData(bool nameOnly) { capHeight = (double)bbox.yMax; } + // If we just wanted to get the font's name and figure out if it's actually usable, close + // it now. If we don't do this, and there are a lot of fonts, we can bump into the file + // descriptor limit (especially on Windows), breaking all file operations. + if(!keepOpen) { + FT_Done_Face(fontFace); + fontFace = NULL; + return true; + } + return true; } @@ -343,8 +347,9 @@ void TtfFont::PlotString(const std::string &str, for(char32_t cid : ReadUTF8(str)) { uint32_t gid = FT_Get_Char_Index(fontFace, cid); if (gid == 0) { - dbp("freetype: CID-to-GID mapping for CID 0x%04x failed: %s; using CID as GID", - cid, ft_error_string(gid)); + dbp("freetype: CID-to-GID mapping for CID 0x%04x in file '%s' failed: %s; " + "using CID as GID", + cid, fontFile.raw.c_str(), ft_error_string(gid)); gid = cid; } @@ -357,8 +362,8 @@ void TtfFont::PlotString(const std::string &str, * ones, antialiasing mitigates this considerably though. */ if(int fterr = FT_Load_Glyph(fontFace, gid, FT_LOAD_NO_BITMAP | FT_LOAD_NO_HINTING)) { - dbp("freetype: cannot load glyph for GID 0x%04x: %s", - gid, ft_error_string(fterr)); + dbp("freetype: cannot load glyph for GID 0x%04x in file '%s': %s", + gid, fontFile.raw.c_str(), ft_error_string(fterr)); return; } @@ -390,8 +395,8 @@ void TtfFont::PlotString(const std::string &str, data.factor = (float)(1.0 / capHeight); data.bx = bx; if(int fterr = FT_Outline_Decompose(&fontFace->glyph->outline, &outlineFuncs, &data)) { - dbp("freetype: bezier decomposition failed (gid %d): %s", - gid, ft_error_string(fterr)); + dbp("freetype: bezier decomposition failed for GID 0x%4x in file '%s': %s", + gid, fontFile.raw.c_str(), ft_error_string(fterr)); } // And we're done, so advance our position by the requested advance @@ -408,13 +413,14 @@ double TtfFont::AspectRatio(const std::string &str) { for(char32_t chr : ReadUTF8(str)) { uint32_t gid = FT_Get_Char_Index(fontFace, chr); if (gid == 0) { - dbp("freetype: CID-to-GID mapping for CID 0x%04x failed: %s; using CID as GID", - chr, ft_error_string(gid)); + dbp("freetype: CID-to-GID mapping for CID 0x%04x in file '%s' failed: %s; " + "using CID as GID", + chr, fontFile.raw.c_str(), ft_error_string(gid)); } if(int fterr = FT_Load_Glyph(fontFace, gid, FT_LOAD_NO_BITMAP | FT_LOAD_NO_HINTING)) { - dbp("freetype: cannot load glyph (GID 0x%04x): %s", - gid, ft_error_string(fterr)); + dbp("freetype: cannot load glyph for GID 0x%04x in file '%s': %s", + gid, fontFile.raw.c_str(), ft_error_string(fterr)); break; } diff --git a/src/ttf.h b/src/ttf.h index 0f1ce70f..565e6a9c 100644 --- a/src/ttf.h +++ b/src/ttf.h @@ -20,14 +20,14 @@ public: bool IsResource() const; std::string FontFileBaseName() const; - bool LoadFromFile(FT_LibraryRec_ *fontLibrary, bool nameOnly = true); - bool LoadFromResource(FT_LibraryRec_ *fontLibrary, bool nameOnly = true); + bool LoadFromFile(FT_LibraryRec_ *fontLibrary, bool keepOpen = false); + bool LoadFromResource(FT_LibraryRec_ *fontLibrary, bool keepOpen = false); void PlotString(const std::string &str, SBezierList *sbl, Vector origin, Vector u, Vector v); double AspectRatio(const std::string &str); - bool ExtractTTFData(bool nameOnly); + bool ExtractTTFData(bool keepOpen); }; class TtfFontList { From 690f87cf4a908d89238dccb3165059b913f8dd63 Mon Sep 17 00:00:00 2001 From: whitequark Date: Sat, 23 Nov 2019 16:33:47 +0000 Subject: [PATCH 16/24] On GNU-compatible compilers, avoid embedding paths into binaries. This is helpful for reproducible builds, and also makes it easier to debug binaries built on another system. --- CMakeLists.txt | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index e8e5b9b1..5b24ffcf 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -66,6 +66,14 @@ endif() # common compiler flags +if((CMAKE_CXX_COMPILER_ID STREQUAL "GNU" AND NOT (CMAKE_CXX_COMPILER_VERSION VERSION_LESS 8.0)) OR + CMAKE_CXX_COMPILER_ID MATCHES "Clang") + set(DEBUG_FLAGS "-fdebug-prefix-map=${CMAKE_CURRENT_SOURCE_DIR}=.") + set(DEBUG_FLAGS "${DEBUG_FLAGS} -ffile-prefix-map=${CMAKE_CURRENT_SOURCE_DIR}=.") + set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${DEBUG_FLAGS}") + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${DEBUG_FLAGS}") +endif() + if(MINGW) set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -static-libgcc") set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -static-libgcc -static-libstdc++") From fb536a155da0561192a599c2f55b4e13688a57c7 Mon Sep 17 00:00:00 2001 From: whitequark Date: Sun, 24 Nov 2019 18:55:46 +0000 Subject: [PATCH 17/24] Test for -ffile-prefix-map support instead of comparing versions. Comparing versions is not viable because Apple clang has its own versioning scheme which is also undocumented. --- CMakeLists.txt | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 5b24ffcf..cf23f9df 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -66,12 +66,13 @@ endif() # common compiler flags -if((CMAKE_CXX_COMPILER_ID STREQUAL "GNU" AND NOT (CMAKE_CXX_COMPILER_VERSION VERSION_LESS 8.0)) OR - CMAKE_CXX_COMPILER_ID MATCHES "Clang") - set(DEBUG_FLAGS "-fdebug-prefix-map=${CMAKE_CURRENT_SOURCE_DIR}=.") - set(DEBUG_FLAGS "${DEBUG_FLAGS} -ffile-prefix-map=${CMAKE_CURRENT_SOURCE_DIR}=.") - set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${DEBUG_FLAGS}") - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${DEBUG_FLAGS}") +include(CheckCXXCompilerFlag) + +set(FILE_PREFIX_MAP "-ffile-prefix-map=${CMAKE_CURRENT_SOURCE_DIR}=.") +check_cxx_compiler_flag("${FILE_PREFIX_MAP}" HAS_FILE_PREFIX_MAP) +if(HAS_FILE_PREFIX_MAP) + set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${FILE_PREFIX_MAP}") + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${FILE_PREFIX_MAP}") endif() if(MINGW) From a60d4df179e24561b26556b7fdcc5e8af23dc41b Mon Sep 17 00:00:00 2001 From: Maximilian Federle Date: Mon, 25 Nov 2019 18:28:42 +0100 Subject: [PATCH 18/24] Refactor Travis config - Fix invalid osx_image xcode8.2 - Drop depreceated sudo keyword - Run debian build on bionic image & re-enable building with ui - Split build into separate jobs & stages for better separation of concerns --- .travis.yml | 49 +++++++++++++++++++++------------------ .travis/build-debian.sh | 4 ---- .travis/install-debian.sh | 10 ++++---- .travis/install-macos.sh | 1 + 4 files changed, 33 insertions(+), 31 deletions(-) diff --git a/.travis.yml b/.travis.yml index 93b331bc..9dbf0369 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,24 +1,27 @@ language: c -os: - - linux - - osx -sudo: required -dist: trusty -osx_image: xcode8.2 -install: - - if [[ "$TRAVIS_OS_NAME" == "linux" ]]; then ./.travis/install-debian.sh; fi - - if [[ "$TRAVIS_OS_NAME" == "osx" ]]; then ./.travis/install-macos.sh; fi -script: - - if [[ "$TRAVIS_OS_NAME" == "linux" ]]; then ./.travis/build-debian.sh; fi - # the awk command is a workaround for https://github.com/travis-ci/travis-ci/issues/4704. - - if [[ "$TRAVIS_OS_NAME" == "osx" ]]; then ./.travis/build-macos.sh | awk '/.{0,32}/ {print $0}'; fi -deploy: - - provider: releases - api_key: - secure: dDlkIawHcODlW9B/20/cQCtzeoocvs0hKuNngRKXKqzXLWTRq33oq/B7+39tAixWbmv6exTpijiKrRNFiSCW5Z4iwHLwaRD4XJznxw63e/Hus/dxg2Tvqx7XFpkCz8mT1Z+gZQE5YxAngeZPpI/sZbZtF1UO3yH5eLeeokZ15p26ZskQUPoYuzrTgTzYL3XfpG3F+20rNBawH1ycsCTVD/08/n31d2m3CrKAsbW7er92ek6w4fzKr7NW8WeXjrPJETVpw5fQg1Od3pRGW8dPQaJcvKQEogMp8Mm0ETYd0qigg89/giBz7QwOgmAWQ4dH+DfZH4Ojl//127QztBolMvyDMQBykWrtJoGcij05sT6K2IJr2FHeUBO12MAEdjiVvhQj3DtTzjPiZAHHDBSLWxLKWWhlhHE4pq7g1MQhqXkaAHI2BLNzwLmaowbMT0bECf9yfz6xx18h6XPQFX44oOktraobVALFlyHqeKa8zdcUt22LF6uAL1m5dxL0tny3eXCIPE4UH/RZgua/cHV9G3cUvKQa/QnFSLRhvWVSbGB+7YsHouBJcsUOOW1gmd5442XuC7mpppccRldh+GSxUk6TBJRAx7TeQ0ybDUaoco9MUqp2twv3KreR2+8Q12PDaAhfQVNEGdF3wTm1sShImjCN4VN3eSLlBEbve1QRQXM= - skip_cleanup: true - file: build/SolveSpace.dmg - on: - repo: solvespace/solvespace - tags: true - condition: "$TRAVIS_OS_NAME == osx" +git: + submodules: false +jobs: + include: + - stage: test + name: "Debian" + os: linux + dist: bionic + install: ./.travis/install-debian.sh + script: ./.travis/build-debian.sh + - stage: deploy + name: "OSX" + os: osx + osx_image: xcode8.3 + install: ./.travis/install-macos.sh + # the awk command is a workaround for https://github.com/travis-ci/travis-ci/issues/4704. + script: ./.travis/build-macos.sh | awk '/.{0,32}/ {print $0}' + deploy: + provider: releases + api_key: + secure: dDlkIawHcODlW9B/20/cQCtzeoocvs0hKuNngRKXKqzXLWTRq33oq/B7+39tAixWbmv6exTpijiKrRNFiSCW5Z4iwHLwaRD4XJznxw63e/Hus/dxg2Tvqx7XFpkCz8mT1Z+gZQE5YxAngeZPpI/sZbZtF1UO3yH5eLeeokZ15p26ZskQUPoYuzrTgTzYL3XfpG3F+20rNBawH1ycsCTVD/08/n31d2m3CrKAsbW7er92ek6w4fzKr7NW8WeXjrPJETVpw5fQg1Od3pRGW8dPQaJcvKQEogMp8Mm0ETYd0qigg89/giBz7QwOgmAWQ4dH+DfZH4Ojl//127QztBolMvyDMQBykWrtJoGcij05sT6K2IJr2FHeUBO12MAEdjiVvhQj3DtTzjPiZAHHDBSLWxLKWWhlhHE4pq7g1MQhqXkaAHI2BLNzwLmaowbMT0bECf9yfz6xx18h6XPQFX44oOktraobVALFlyHqeKa8zdcUt22LF6uAL1m5dxL0tny3eXCIPE4UH/RZgua/cHV9G3cUvKQa/QnFSLRhvWVSbGB+7YsHouBJcsUOOW1gmd5442XuC7mpppccRldh+GSxUk6TBJRAx7TeQ0ybDUaoco9MUqp2twv3KreR2+8Q12PDaAhfQVNEGdF3wTm1sShImjCN4VN3eSLlBEbve1QRQXM= + skip_cleanup: true + file: build/SolveSpace.dmg + on: + repo: solvespace/solvespace + tags: true diff --git a/.travis/build-debian.sh b/.travis/build-debian.sh index 03e83e30..bb678daf 100755 --- a/.travis/build-debian.sh +++ b/.travis/build-debian.sh @@ -4,12 +4,8 @@ if echo $TRAVIS_TAG | grep ^v; then BUILD_TYPE=RelWithDebInfo; else BUILD_TYPE=D mkdir build cd build -# We build without the GUI until Travis updates to an Ubuntu version with GTK 3.16+. cmake .. \ - -DCMAKE_C_COMPILER=gcc-5 \ - -DCMAKE_CXX_COMPILER=g++-5 \ -DCMAKE_BUILD_TYPE=$BUILD_TYPE \ - -DENABLE_GUI=OFF \ -DENABLE_SANITIZERS=ON make VERBOSE=1 make test_solvespace diff --git a/.travis/install-debian.sh b/.travis/install-debian.sh index 6b23cc8b..c8b7d70b 100755 --- a/.travis/install-debian.sh +++ b/.travis/install-debian.sh @@ -1,8 +1,10 @@ #!/bin/sh -xe -sudo add-apt-repository -y ppa:ubuntu-toolchain-r/test sudo apt-get update -qq + sudo apt-get install -q -y \ - cmake cmake-data libc6-dev libpng12-dev zlib1g-dev libjson0-dev libfontconfig1-dev \ - libgtkmm-3.0-dev libpangomm-1.4-dev libcairo2-dev libgl1-mesa-dev libglu-dev \ - libfreetype6-dev dpkg-dev gcc-5 g++-5 lcov + zlib1g-dev libpng-dev libcairo2-dev libfreetype6-dev libjson-c-dev \ + libfontconfig1-dev libgtkmm-3.0-dev libpangomm-1.4-dev libgl-dev \ + libgl-dev libglu-dev libspnav-dev + +git submodule update --init extlib/libdxfrw extlib/flatbuffers extlib/q3d diff --git a/.travis/install-macos.sh b/.travis/install-macos.sh index 5d434d29..792126ca 100755 --- a/.travis/install-macos.sh +++ b/.travis/install-macos.sh @@ -2,3 +2,4 @@ brew update brew install freetype cairo +git submodule update --init From 07992cecaaeeeb0584f3c655eaaf32ba09bd3e6d Mon Sep 17 00:00:00 2001 From: whitequark Date: Sun, 24 Nov 2019 22:29:45 +0000 Subject: [PATCH 19/24] Remove unused offscreen GL renderer. This was used for Gtk 2 and old macOS, but all of those use direct rendering now. --- src/render/render.h | 16 ------------ src/render/rendergl.cpp | 58 ----------------------------------------- 2 files changed, 74 deletions(-) delete mode 100644 src/render/rendergl.cpp diff --git a/src/render/render.h b/src/render/render.h index 649256e7..70d5863f 100644 --- a/src/render/render.h +++ b/src/render/render.h @@ -375,22 +375,6 @@ public: std::shared_ptr ReadFrame() override; }; -//----------------------------------------------------------------------------- -// 3d renderers -//----------------------------------------------------------------------------- - -// An offscreen renderer based on OpenGL framebuffers. -class GlOffscreen { -public: - unsigned int framebuffer = 0; - unsigned int colorRenderbuffer = 0; - unsigned int depthRenderbuffer = 0; - std::vector data; - - bool Render(int width, int height, std::function renderFn); - void Clear(); -}; - //----------------------------------------------------------------------------- // Factories //----------------------------------------------------------------------------- diff --git a/src/render/rendergl.cpp b/src/render/rendergl.cpp deleted file mode 100644 index 38a8fbf2..00000000 --- a/src/render/rendergl.cpp +++ /dev/null @@ -1,58 +0,0 @@ -//----------------------------------------------------------------------------- -// Offscreen rendering in OpenGL using EGL and framebuffer objects. -// -// Copyright 2015-2016 whitequark -//----------------------------------------------------------------------------- -#ifdef __APPLE__ -#include -#else -#define GL_GLEXT_PROTOTYPES -#include -#endif - -#include "solvespace.h" - -void GlOffscreen::Clear() { - glDeleteRenderbuffersEXT(1, &depthRenderbuffer); - glDeleteRenderbuffersEXT(1, &colorRenderbuffer); - glDeleteFramebuffersEXT(1, &framebuffer); - *this = {}; -} - -bool GlOffscreen::Render(int width, int height, std::function renderFn) { - data.resize(width * height * 4); - - if(framebuffer == 0) - glGenFramebuffersEXT(1, &framebuffer); - if(colorRenderbuffer == 0) - glGenRenderbuffersEXT(1, &colorRenderbuffer); - if(depthRenderbuffer == 0) - glGenRenderbuffersEXT(1, &depthRenderbuffer); - - glBindFramebufferEXT(GL_FRAMEBUFFER_EXT, framebuffer); - - glBindRenderbufferEXT(GL_RENDERBUFFER_EXT, colorRenderbuffer); - glRenderbufferStorageEXT(GL_RENDERBUFFER_EXT, GL_RGBA8, width, height); - glFramebufferRenderbufferEXT(GL_FRAMEBUFFER_EXT, GL_COLOR_ATTACHMENT0_EXT, - GL_RENDERBUFFER_EXT, colorRenderbuffer); - - glBindRenderbufferEXT(GL_RENDERBUFFER_EXT, depthRenderbuffer); - glRenderbufferStorageEXT(GL_RENDERBUFFER_EXT, GL_DEPTH_COMPONENT24, width, height); - glFramebufferRenderbufferEXT(GL_FRAMEBUFFER_EXT, GL_DEPTH_ATTACHMENT_EXT, - GL_RENDERBUFFER_EXT, depthRenderbuffer); - - bool framebufferComplete = - glCheckFramebufferStatusEXT(GL_FRAMEBUFFER_EXT) == GL_FRAMEBUFFER_COMPLETE_EXT; - if(framebufferComplete) { - renderFn(); -#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__ - glReadPixels(0, 0, width, height, GL_BGRA, GL_UNSIGNED_INT_8_8_8_8_REV, &data[0]); -#else - glReadPixels(0, 0, width, height, GL_BGRA, GL_UNSIGNED_INT_8_8_8_8, &data[0]); -#endif - } - - glBindFramebufferEXT(GL_FRAMEBUFFER_EXT, 0); - - return framebufferComplete; -} From 22525e65157ebc1628b682988f447fcd3199dfd8 Mon Sep 17 00:00:00 2001 From: whitequark Date: Mon, 25 Nov 2019 10:56:38 +0000 Subject: [PATCH 20/24] CMake: update policy to 3.11. In particular this enables linking with GLVND on Linux by default. --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index cf23f9df..9bfd04d0 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -8,7 +8,7 @@ if(CMAKE_SOURCE_DIR STREQUAL CMAKE_BINARY_DIR) endif() cmake_minimum_required(VERSION 3.2.0 FATAL_ERROR) -cmake_policy(VERSION 3.2.0) +cmake_policy(VERSION 3.11) set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} "${CMAKE_SOURCE_DIR}/cmake/") set(CMAKE_CXX_STANDARD 11) From 0501f0c99e75ff0e6c5d07d1cd7bcdc119f6b3af Mon Sep 17 00:00:00 2001 From: whitequark Date: Mon, 25 Nov 2019 10:51:29 +0000 Subject: [PATCH 21/24] Don't call GL functions in OpenGl3Renderer::GetIdent. 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. --- src/render/rendergl3.cpp | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/render/rendergl3.cpp b/src/render/rendergl3.cpp index b3ae1fa6..b620aee0 100644 --- a/src/render/rendergl3.cpp +++ b/src/render/rendergl3.cpp @@ -87,6 +87,9 @@ public: Fill *fill; std::weak_ptr texture; } current; + const char *vendor = ""; + const char *renderer = ""; + const char *version = ""; // List-initialize current to work around MSVC bug 746973. OpenGl3Renderer() : @@ -440,6 +443,10 @@ void OpenGl3Renderer::Init() { meshRenderer.Init(); imeshRenderer.Init(); + vendor = (const char *)glGetString(GL_VENDOR); + renderer = (const char *)glGetString(GL_RENDERER); + version = (const char *)glGetString(GL_VERSION); + #if !defined(HAVE_GLES) && !defined(__APPLE__) GLuint array; glGenVertexArrays(1, &array); @@ -696,9 +703,9 @@ std::shared_ptr OpenGl3Renderer::ReadFrame() { } void OpenGl3Renderer::GetIdent(const char **vendor, const char **renderer, const char **version) { - *vendor = (const char *)glGetString(GL_VENDOR); - *renderer = (const char *)glGetString(GL_RENDERER); - *version = (const char *)glGetString(GL_VERSION); + *vendor = this->vendor; + *renderer = this->renderer; + *version = this->version; } void OpenGl3Renderer::SetCamera(const Camera &c) { From 552fe354aae77ce60416ede35ef4e9852290b67f Mon Sep 17 00:00:00 2001 From: Koen Schmeets Date: Tue, 26 Nov 2019 15:34:30 +0100 Subject: [PATCH 22/24] Fix commit 07992cecaa. --- src/CMakeLists.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index f53d4e2f..36fdc5f5 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -128,7 +128,6 @@ elseif(APPLE) -fobjc-arc) list(APPEND platform_SOURCES - render/rendergl.cpp platform/guimac.mm) else() list(APPEND platform_SOURCES From 58f23aa061721407a2b539ca8bdf20b40e96211d Mon Sep 17 00:00:00 2001 From: Koen Schmeets Date: Tue, 26 Nov 2019 21:31:14 +0100 Subject: [PATCH 23/24] macOS: force show property browser when opening preferences from menu. --- src/platform/guimac.mm | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/platform/guimac.mm b/src/platform/guimac.mm index 33901ff6..a5763c90 100644 --- a/src/platform/guimac.mm +++ b/src/platform/guimac.mm @@ -1409,6 +1409,9 @@ void OpenInBrowser(const std::string &url) { @implementation SSApplicationDelegate - (IBAction)preferences:(id)sender { + if (!SS.GW.showTextWindow) { + SolveSpace::SS.GW.MenuView(SolveSpace::Command::SHOW_TEXT_WND); + } SolveSpace::SS.TW.GoToScreen(SolveSpace::TextWindow::Screen::CONFIGURATION); SolveSpace::SS.ScheduleShowTW(); } From dcdfdec564fc2ab1e087cdc56d017162a05cf778 Mon Sep 17 00:00:00 2001 From: Koen Schmeets Date: Tue, 26 Nov 2019 23:34:53 +0100 Subject: [PATCH 24/24] Add an option to edit dimension immediately after adding. --- CHANGELOG.md | 2 + src/confscreen.cpp | 8 +++ src/constraint.cpp | 6 +++ src/mouse.cpp | 128 +++++++++++++++++++++++---------------------- src/solvespace.cpp | 4 ++ src/solvespace.h | 1 + src/ui.h | 2 + 7 files changed, 89 insertions(+), 62 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b7f6d7a2..9787a9cd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,8 @@ New constraint features: constraints on line segments. * Automatic creation of constraints no longer happens if the constraint would have been redundant with other ones. + * New option to open the constraint editor for newly created constraints + with a value. New export/import features: * Three.js: allow configuring projection for exported model, and initially diff --git a/src/confscreen.cpp b/src/confscreen.cpp index 6479fccc..646f91e0 100644 --- a/src/confscreen.cpp +++ b/src/confscreen.cpp @@ -102,6 +102,11 @@ void TextWindow::ScreenChangeTurntableNav(int link, uint32_t v) { } } +void TextWindow::ScreenChangeImmediatelyEditDimension(int link, uint32_t v) { + SS.immediatelyEditDimension = !SS.immediatelyEditDimension; + SS.GW.Invalidate(/*clearPersistent=*/true); +} + void TextWindow::ScreenChangeShowContourAreas(int link, uint32_t v) { SS.showContourAreas = !SS.showContourAreas; SS.GW.Invalidate(); @@ -342,6 +347,9 @@ void TextWindow::ShowConfiguration() { SS.automaticLineConstraints ? CHECK_TRUE : CHECK_FALSE); Printf(false, " %Fd%f%Ll%s use turntable mouse navigation%E", &ScreenChangeTurntableNav, SS.turntableNav ? CHECK_TRUE : CHECK_FALSE); + Printf(false, " %Fd%f%Ll%s edit newly added dimensions%E", + &ScreenChangeImmediatelyEditDimension, + SS.immediatelyEditDimension ? CHECK_TRUE : CHECK_FALSE); Printf(false, ""); Printf(false, "%Ft autosave interval (in minutes)%E"); Printf(false, "%Ba %d %Fl%Ll%f[change]%E", diff --git a/src/constraint.cpp b/src/constraint.cpp index 8ac09d49..8ab76f27 100644 --- a/src/constraint.cpp +++ b/src/constraint.cpp @@ -196,6 +196,9 @@ void Constraint::MenuConstrain(Command id) { c.valA = 0; c.ModifyToSatisfy(); AddConstraint(&c); + if (SS.immediatelyEditDimension) { + SS.GW.EditConstraint(c.h); + } break; } @@ -607,6 +610,9 @@ void Constraint::MenuConstrain(Command id) { c.ModifyToSatisfy(); AddConstraint(&c); + if (SS.immediatelyEditDimension) { + SS.GW.EditConstraint(c.h); + } break; } diff --git a/src/mouse.cpp b/src/mouse.cpp index 0e101a78..c0d1798e 100644 --- a/src/mouse.cpp +++ b/src/mouse.cpp @@ -1339,73 +1339,77 @@ void GraphicsWindow::MouseLeftUp(double mx, double my, bool shiftDown, bool ctrl } } +void GraphicsWindow::EditConstraint(hConstraint constraint) { + constraintBeingEdited = constraint; + ClearSuper(); + + Constraint *c = SK.GetConstraint(constraintBeingEdited); + if(!c->HasLabel()) { + // Not meaningful to edit a constraint without a dimension + return; + } + if(c->reference) { + // Not meaningful to edit a reference dimension + return; + } + + Vector p3 = c->GetLabelPos(GetCamera()); + Point2d p2 = ProjectPoint(p3); + + std::string editValue; + std::string editPlaceholder; + switch(c->type) { + case Constraint::Type::COMMENT: + editValue = c->comment; + editPlaceholder = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; + break; + + default: { + double value = fabs(c->valA); + + // If displayed as radius, also edit as radius. + if(c->type == Constraint::Type::DIAMETER && c->other) + value /= 2; + + // Try showing value with default number of digits after decimal first. + if(c->type == Constraint::Type::LENGTH_RATIO) { + editValue = ssprintf("%.3f", value); + } else if(c->type == Constraint::Type::ANGLE) { + editValue = SS.DegreeToString(value); + } else { + editValue = SS.MmToString(value); + value /= SS.MmPerUnit(); + } + // If that's not enough to represent it exactly, show the value with as many + // digits after decimal as required, up to 10. + int digits = 0; + while(fabs(std::stod(editValue) - value) > 1e-10) { + editValue = ssprintf("%.*f", digits, value); + digits++; + } + editPlaceholder = "10.000000"; + break; + } + } + + double width, height; + window->GetContentSize(&width, &height); + hStyle hs = c->disp.style; + if(hs.v == 0) hs.v = Style::CONSTRAINT; + double capHeight = Style::TextHeight(hs); + double fontHeight = VectorFont::Builtin()->GetHeight(capHeight); + double editMinWidth = VectorFont::Builtin()->GetWidth(capHeight, editPlaceholder); + window->ShowEditor(p2.x + width / 2, height / 2 - p2.y, + fontHeight, editMinWidth, + /*isMonospace=*/false, editValue); +} + void GraphicsWindow::MouseLeftDoubleClick(double mx, double my) { if(window->IsEditorVisible()) return; SS.TW.HideEditControl(); if(hover.constraint.v) { - constraintBeingEdited = hover.constraint; - ClearSuper(); - - Constraint *c = SK.GetConstraint(constraintBeingEdited); - if(!c->HasLabel()) { - // Not meaningful to edit a constraint without a dimension - return; - } - if(c->reference) { - // Not meaningful to edit a reference dimension - return; - } - - Vector p3 = c->GetLabelPos(GetCamera()); - Point2d p2 = ProjectPoint(p3); - - std::string editValue; - std::string editPlaceholder; - switch(c->type) { - case Constraint::Type::COMMENT: - editValue = c->comment; - editPlaceholder = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; - break; - - default: { - double value = fabs(c->valA); - - // If displayed as radius, also edit as radius. - if(c->type == Constraint::Type::DIAMETER && c->other) - value /= 2; - - // Try showing value with default number of digits after decimal first. - if(c->type == Constraint::Type::LENGTH_RATIO) { - editValue = ssprintf("%.3f", value); - } else if(c->type == Constraint::Type::ANGLE) { - editValue = SS.DegreeToString(value); - } else { - editValue = SS.MmToString(value); - value /= SS.MmPerUnit(); - } - // If that's not enough to represent it exactly, show the value with as many - // digits after decimal as required, up to 10. - int digits = 0; - while(fabs(std::stod(editValue) - value) > 1e-10) { - editValue = ssprintf("%.*f", digits, value); - digits++; - } - editPlaceholder = "10.000000"; - break; - } - } - - double width, height; - window->GetContentSize(&width, &height); - hStyle hs = c->disp.style; - if(hs.v == 0) hs.v = Style::CONSTRAINT; - double capHeight = Style::TextHeight(hs); - double fontHeight = VectorFont::Builtin()->GetHeight(capHeight); - double editMinWidth = VectorFont::Builtin()->GetWidth(capHeight, editPlaceholder); - window->ShowEditor(p2.x + width / 2, height / 2 - p2.y, - fontHeight, editMinWidth, - /*isMonospace=*/false, editValue); + EditConstraint(hover.constraint); } } diff --git a/src/solvespace.cpp b/src/solvespace.cpp index 367ce09e..1a1df147 100644 --- a/src/solvespace.cpp +++ b/src/solvespace.cpp @@ -72,6 +72,8 @@ void SolveSpaceUI::Init() { drawBackFaces = settings->ThawBool("DrawBackFaces", true); // Use turntable mouse navigation turntableNav = settings->ThawBool("TurntableNav", false); + // Immediately edit dimension + immediatelyEditDimension = settings->ThawBool("ImmediatelyEditDimension", false); // Check that contours are closed and not self-intersecting checkClosedContour = settings->ThawBool("CheckClosedContour", true); // Enable automatic constrains for lines @@ -251,6 +253,8 @@ void SolveSpaceUI::Exit() { settings->FreezeBool("CheckClosedContour", checkClosedContour); // Use turntable mouse navigation settings->FreezeBool("TurntableNav", turntableNav); + // Immediately edit dimensions + settings->FreezeBool("ImmediatelyEditDimension", immediatelyEditDimension); // Enable automatic constrains for lines settings->FreezeBool("AutomaticLineConstraints", automaticLineConstraints); // Export shaded triangles in a 2d view diff --git a/src/solvespace.h b/src/solvespace.h index 64397414..be0284cd 100644 --- a/src/solvespace.h +++ b/src/solvespace.h @@ -587,6 +587,7 @@ public: bool showContourAreas; bool checkClosedContour; bool turntableNav; + bool immediatelyEditDimension; bool automaticLineConstraints; bool showToolbar; Platform::Path screenshotFile; diff --git a/src/ui.h b/src/ui.h index a2985598..7d24e84d 100644 --- a/src/ui.h +++ b/src/ui.h @@ -430,6 +430,7 @@ public: static void ScreenChangeShowContourAreas(int link, uint32_t v); static void ScreenChangeCheckClosedContour(int link, uint32_t v); static void ScreenChangeTurntableNav(int link, uint32_t v); + static void ScreenChangeImmediatelyEditDimension(int link, uint32_t v); static void ScreenChangeAutomaticLineConstraints(int link, uint32_t v); static void ScreenChangePwlCurves(int link, uint32_t v); static void ScreenChangeCanvasSizeAuto(int link, uint32_t v); @@ -690,6 +691,7 @@ public: void RemoveConstraintsForPointBeingDeleted(hEntity hpt); void FixConstraintsForRequestBeingDeleted(hRequest hr); void FixConstraintsForPointBeingDeleted(hEntity hpt); + void EditConstraint(hConstraint constraint); // A selected entity. class Selection {