{"action":"created","comment":{"url":"https://api.github.com/repos/kduske/TrenchBroom/pulls/comments/199327678","pull_request_review_id":128933991,"id":199327678,"node_id":"MDI0OlB1bGxSZXF1ZXN0UmV2aWV3Q29tbWVudDE5OTMyNzY3OA==","diff_hunk":"@@ -1411,5 +1416,102 @@ namespace TrenchBroom {\n             assert(intersects.hasResult());\n             return intersects.result();\n         }\n+\n+        void Brush::invalidateVertexCache() {\n+            m_rendererCacheValid = false;\n+            m_cachedVertices.clear();\n+            m_cachedEdges.clear();\n+            m_cachedFacesSortedByTexture.clear();\n+        }\n+\n+        void Brush::validateVertexCache() const {\n+            if (m_rendererCacheValid)\n+                return;\n+\n+            ensure(m_geometry != nullptr, \"geometry is null\");\n+\n+            // build vertex cache and face cache\n+\n+            m_cachedVertices.clear();\n+            m_cachedVertices.reserve(vertexCount());\n+\n+            m_cachedFacesSortedByTexture.clear();\n+            m_cachedFacesSortedByTexture.reserve(faceCount());\n+\n+            for (BrushFace* face : m_faces) {\n+                const size_t startIndex = m_cachedVertices.size();\n+\n+                const BrushHalfEdge *first = face->geometry()->boundary().front();\n+                const BrushHalfEdge *current = first;\n+                do {\n+                    BrushVertex* vertex = current->origin();\n+\n+                    // Set the vertex payload to the index, relative to the brush's first vertex being 0.\n+                    // This is used below when building the edge cache.\n+                    // NOTE: we'll overwrite the payload as we visit the same vertex several times while visiting\n+                    // different faces, this is fine.\n+                    const size_t currentIndex = m_cachedVertices.size();\n+                    vertex->setPayload(static_cast<GLuint>(currentIndex));\n+\n+                    const Vec3 &position = vertex->position();","path":"common/src/Model/Brush.cpp","position":83,"original_position":83,"commit_id":"41e9a345144083935fd39a4ef284466d8acae845","original_commit_id":"41e9a345144083935fd39a4ef284466d8acae845","user":{"login":"kduske","id":186061,"node_id":"MDQ6VXNlcjE4NjA2MQ==","avatar_url":"https://avatars3.githubusercontent.com/u/186061?v=4","gravatar_id":"","url":"https://api.github.com/users/kduske","html_url":"https://github.com/kduske","followers_url":"https://api.github.com/users/kduske/followers","following_url":"https://api.github.com/users/kduske/following{/other_user}","gists_url":"https://api.github.com/users/kduske/gists{/gist_id}","starred_url":"https://api.github.com/users/kduske/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/kduske/subscriptions","organizations_url":"https://api.github.com/users/kduske/orgs","repos_url":"https://api.github.com/users/kduske/repos","events_url":"https://api.github.com/users/kduske/events{/privacy}","received_events_url":"https://api.github.com/users/kduske/received_events","type":"User","site_admin":false},"body":"please use `const Vec3& position` -- I'd like to keep style consistent if possible.","created_at":"2018-06-30T17:21:54Z","updated_at":"2018-06-30T17:40:04Z","html_url":"https://github.com/kduske/TrenchBroom/pull/2151#discussion_r199327678","pull_request_url":"https://api.github.com/repos/kduske/TrenchBroom/pulls/2151","author_association":"OWNER","_links":{"self":{"href":"https://api.github.com/repos/kduske/TrenchBroom/pulls/comments/199327678"},"html":{"href":"https://github.com/kduske/TrenchBroom/pull/2151#discussion_r199327678"},"pull_request":{"href":"https://api.github.com/repos/kduske/TrenchBroom/pulls/2151"}}},"pull_request":{"url":"https://api.github.com/repos/kduske/TrenchBroom/pulls/2151","id":194829521,"node_id":"MDExOlB1bGxSZXF1ZXN0MTk0ODI5NTIx","html_url":"https://github.com/kduske/TrenchBroom/pull/2151","diff_url":"https://github.com/kduske/TrenchBroom/pull/2151.diff","patch_url":"https://github.com/kduske/TrenchBroom/pull/2151.patch","issue_url":"https://api.github.com/repos/kduske/TrenchBroom/issues/2151","number":2151,"state":"open","locked":false,"title":"1404: BrushRenderer rewrite with partial VBO updates","user":{"login":"ericwa","id":239161,"node_id":"MDQ6VXNlcjIzOTE2MQ==","avatar_url":"https://avatars1.githubusercontent.com/u/239161?v=4","gravatar_id":"","url":"https://api.github.com/users/ericwa","html_url":"https://github.com/ericwa","followers_url":"https://api.github.com/users/ericwa/followers","following_url":"https://api.github.com/users/ericwa/following{/other_user}","gists_url":"https://api.github.com/users/ericwa/gists{/gist_id}","starred_url":"https://api.github.com/users/ericwa/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ericwa/subscriptions","organizations_url":"https://api.github.com/users/ericwa/orgs","repos_url":"https://api.github.com/users/ericwa/repos","events_url":"https://api.github.com/users/ericwa/events{/privacy}","received_events_url":"https://api.github.com/users/ericwa/received_events","type":"User","site_admin":false},"body":"This implements the \"partial vbo updates\" idea I have been working on for a while.\r\n\r\nThe worst-case performance (validating all of 64K brushes) is around 30% better than master, which is still not great, but when making small changes in selections it's really fast. For removing one brush out of 64K from the renderer, it's about 22ms, compared to 300ms on master, and most of the 22ms is spent diffing the sets of brushes in BrushRenderer::setBrushes, which can be improved.\r\n\r\n- BrushRenderer now updates its VBO's one brush at a time (adding or removing brushes). There's one VboBlock for brush vertices, one for brush edges, and one per texture for brush face triangles. Each of these VboBlock has an AllocationTracker, which is a dynamic memory allocator (turned out very similar to Vbo/VboBlock's). When adding a brush, the AllocationTrackers find a free chunk of the needed VBO's, and BrushRenderer records where the vertices/indices were stored (BrushRenderer::BrushInfo). When brushes are deleted, the indices are zeroed so they become degenerate lines/tris and are ignored by OpenGL, so the unused space in the index arrays left by deleted brushes don't cause any problems (and the AllocationTracker will handle re-using the space for future allocations.)\r\n\r\n- Some adjustments to MapRenderer to work with the new BrushRenderer. e.g. MapRenderer::selectionDidChange now needs to invalidate the brushes which had faces selected/unselected. \r\n\r\n- BrushRenderer::Filter API is rewritten, primarily for performance (it's a bit less flexible, but still flexible enough to handle everything TB was doing - you can't give a custom predicate for deciding whether to render an edge anymore), and there is no more redundant evaluation of brush visibility / locked state.\r\n\r\n- The BrushFace rendering cache is moved to Brush.\r\n\r\nOne area that could be improved is the MapRenderer integration.\r\ne.g. I had to change MapRenderer::nodeVisibilityDidChange to call `invalidateRenderers(Renderer_All);` to fix the Show All command (updateRenderers doesn't re-evaluate brush fitlers now), but this will be slow, and maybe I can just invalidate the changed brushes now.","created_at":"2018-06-14T09:23:45Z","updated_at":"2018-06-30T17:40:04Z","closed_at":null,"merged_at":null,"merge_commit_sha":"dd331d91122482a12122e188ee6185f630041d78","assignee":{"login":"ericwa","id":239161,"node_id":"MDQ6VXNlcjIzOTE2MQ==","avatar_url":"https://avatars1.githubusercontent.com/u/239161?v=4","gravatar_id":"","url":"https://api.github.com/users/ericwa","html_url":"https://github.com/ericwa","followers_url":"https://api.github.com/users/ericwa/followers","following_url":"https://api.github.com/users/ericwa/following{/other_user}","gists_url":"https://api.github.com/users/ericwa/gists{/gist_id}","starred_url":"https://api.github.com/users/ericwa/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ericwa/subscriptions","organizations_url":"https://api.github.com/users/ericwa/orgs","repos_url":"https://api.github.com/users/ericwa/repos","events_url":"https://api.github.com/users/ericwa/events{/privacy}","received_events_url":"https://api.github.com/users/ericwa/received_events","type":"User","site_admin":false},"assignees":[{"login":"ericwa","id":239161,"node_id":"MDQ6VXNlcjIzOTE2MQ==","avatar_url":"https://avatars1.githubusercontent.com/u/239161?v=4","gravatar_id":"","url":"https://api.github.com/users/ericwa","html_url":"https://github.com/ericwa","followers_url":"https://api.github.com/users/ericwa/followers","following_url":"https://api.github.com/users/ericwa/following{/other_user}","gists_url":"https://api.github.com/users/ericwa/gists{/gist_id}","starred_url":"https://api.github.com/users/ericwa/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ericwa/subscriptions","organizations_url":"https://api.github.com/users/ericwa/orgs","repos_url":"https://api.github.com/users/ericwa/repos","events_url":"https://api.github.com/users/ericwa/events{/privacy}","received_events_url":"https://api.github.com/users/ericwa/received_events","type":"User","site_admin":false}],"requested_reviewers":[],"requested_teams":[],"labels":[{"id":507324566,"node_id":"MDU6TGFiZWw1MDczMjQ1NjY=","url":"https://api.github.com/repos/kduske/TrenchBroom/labels/Status:In%20Progress","name":"Status:In Progress","color":"bfd4f2","default":false}],"milestone":null,"commits_url":"https://api.github.com/repos/kduske/TrenchBroom/pulls/2151/commits","review_comments_url":"https://api.github.com/repos/kduske/TrenchBroom/pulls/2151/comments","review_comment_url":"https://api.github.com/repos/kduske/TrenchBroom/pulls/comments{/number}","comments_url":"https://api.github.com/repos/kduske/TrenchBroom/issues/2151/comments","statuses_url":"https://api.github.com/repos/kduske/TrenchBroom/statuses/41e9a345144083935fd39a4ef284466d8acae845","head":{"label":"kduske:feature/1404_partial_vbo_updates","ref":"feature/1404_partial_vbo_updates","sha":"41e9a345144083935fd39a4ef284466d8acae845","user":{"login":"kduske","id":186061,"node_id":"MDQ6VXNlcjE4NjA2MQ==","avatar_url":"https://avatars3.githubusercontent.com/u/186061?v=4","gravatar_id":"","url":"https://api.github.com/users/kduske","html_url":"https://github.com/kduske","followers_url":"https://api.github.com/users/kduske/followers","following_url":"https://api.github.com/users/kduske/following{/other_user}","gists_url":"https://api.github.com/users/kduske/gists{/gist_id}","starred_url":"https://api.github.com/users/kduske/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/kduske/subscriptions","organizations_url":"https://api.github.com/users/kduske/orgs","repos_url":"https://api.github.com/users/kduske/repos","events_url":"https://api.github.com/users/kduske/events{/privacy}","received_events_url":"https://api.github.com/users/kduske/received_events","type":"User","site_admin":false},"repo":{"id":4003232,"node_id":"MDEwOlJlcG9zaXRvcnk0MDAzMjMy","name":"TrenchBroom","full_name":"kduske/TrenchBroom","owner":{"login":"kduske","id":186061,"node_id":"MDQ6VXNlcjE4NjA2MQ==","avatar_url":"https://avatars3.githubusercontent.com/u/186061?v=4","gravatar_id":"","url":"https://api.github.com/users/kduske","html_url":"https://github.com/kduske","followers_url":"https://api.github.com/users/kduske/followers","following_url":"https://api.github.com/users/kduske/following{/other_user}","gists_url":"https://api.github.com/users/kduske/gists{/gist_id}","starred_url":"https://api.github.com/users/kduske/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/kduske/subscriptions","organizations_url":"https://api.github.com/users/kduske/orgs","repos_url":"https://api.github.com/users/kduske/repos","events_url":"https://api.github.com/users/kduske/events{/privacy}","received_events_url":"https://api.github.com/users/kduske/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/kduske/TrenchBroom","description":"Cross-Platform Level Editor","fork":false,"url":"https://api.github.com/repos/kduske/TrenchBroom","forks_url":"https://api.github.com/repos/kduske/TrenchBroom/forks","keys_url":"https://api.github.com/repos/kduske/TrenchBroom/keys{/key_id}","collaborators_url":"https://api.github.com/repos/kduske/TrenchBroom/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/kduske/TrenchBroom/teams","hooks_url":"https://api.github.com/repos/kduske/TrenchBroom/hooks","issue_events_url":"https://api.github.com/repos/kduske/TrenchBroom/issues/events{/number}","events_url":"https://api.github.com/repos/kduske/TrenchBroom/events","assignees_url":"https://api.github.com/repos/kduske/TrenchBroom/assignees{/user}","branches_url":"https://api.github.com/repos/kduske/TrenchBroom/branches{/branch}","tags_url":"https://api.github.com/repos/kduske/TrenchBroom/tags","blobs_url":"https://api.github.com/repos/kduske/TrenchBroom/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/kduske/TrenchBroom/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/kduske/TrenchBroom/git/refs{/sha}","trees_url":"https://api.github.com/repos/kduske/TrenchBroom/git/trees{/sha}","statuses_url":"https://api.github.com/repos/kduske/TrenchBroom/statuses/{sha}","languages_url":"https://api.github.com/repos/kduske/TrenchBroom/languages","stargazers_url":"https://api.github.com/repos/kduske/TrenchBroom/stargazers","contributors_url":"https://api.github.com/repos/kduske/TrenchBroom/contributors","subscribers_url":"https://api.github.com/repos/kduske/TrenchBroom/subscribers","subscription_url":"https://api.github.com/repos/kduske/TrenchBroom/subscription","commits_url":"https://api.github.com/repos/kduske/TrenchBroom/commits{/sha}","git_commits_url":"https://api.github.com/repos/kduske/TrenchBroom/git/commits{/sha}","comments_url":"https://api.github.com/repos/kduske/TrenchBroom/comments{/number}","issue_comment_url":"https://api.github.com/repos/kduske/TrenchBroom/issues/comments{/number}","contents_url":"https://api.github.com/repos/kduske/TrenchBroom/contents/{+path}","compare_url":"https://api.github.com/repos/kduske/TrenchBroom/compare/{base}...{head}","merges_url":"https://api.github.com/repos/kduske/TrenchBroom/merges","archive_url":"https://api.github.com/repos/kduske/TrenchBroom/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/kduske/TrenchBroom/downloads","issues_url":"https://api.github.com/repos/kduske/TrenchBroom/issues{/number}","pulls_url":"https://api.github.com/repos/kduske/TrenchBroom/pulls{/number}","milestones_url":"https://api.github.com/repos/kduske/TrenchBroom/milestones{/number}","notifications_url":"https://api.github.com/repos/kduske/TrenchBroom/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/kduske/TrenchBroom/labels{/name}","releases_url":"https://api.github.com/repos/kduske/TrenchBroom/releases{/id}","deployments_url":"https://api.github.com/repos/kduske/TrenchBroom/deployments","created_at":"2012-04-12T09:39:51Z","updated_at":"2018-06-24T23:47:41Z","pushed_at":"2018-06-26T20:47:08Z","git_url":"git://github.com/kduske/TrenchBroom.git","ssh_url":"git@github.com:kduske/TrenchBroom.git","clone_url":"https://github.com/kduske/TrenchBroom.git","svn_url":"https://github.com/kduske/TrenchBroom","homepage":"kristianduske.com/trenchbroom","size":112874,"stargazers_count":352,"watchers_count":352,"language":"C++","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":43,"mirror_url":null,"archived":false,"open_issues_count":186,"license":null,"forks":43,"open_issues":186,"watchers":352,"default_branch":"master"}},"base":{"label":"kduske:master","ref":"master","sha":"ed4b9b12d4afcb6240351daf74295ccc33ace1de","user":{"login":"kduske","id":186061,"node_id":"MDQ6VXNlcjE4NjA2MQ==","avatar_url":"https://avatars3.githubusercontent.com/u/186061?v=4","gravatar_id":"","url":"https://api.github.com/users/kduske","html_url":"https://github.com/kduske","followers_url":"https://api.github.com/users/kduske/followers","following_url":"https://api.github.com/users/kduske/following{/other_user}","gists_url":"https://api.github.com/users/kduske/gists{/gist_id}","starred_url":"https://api.github.com/users/kduske/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/kduske/subscriptions","organizations_url":"https://api.github.com/users/kduske/orgs","repos_url":"https://api.github.com/users/kduske/repos","events_url":"https://api.github.com/users/kduske/events{/privacy}","received_events_url":"https://api.github.com/users/kduske/received_events","type":"User","site_admin":false},"repo":{"id":4003232,"node_id":"MDEwOlJlcG9zaXRvcnk0MDAzMjMy","name":"TrenchBroom","full_name":"kduske/TrenchBroom","owner":{"login":"kduske","id":186061,"node_id":"MDQ6VXNlcjE4NjA2MQ==","avatar_url":"https://avatars3.githubusercontent.com/u/186061?v=4","gravatar_id":"","url":"https://api.github.com/users/kduske","html_url":"https://github.com/kduske","followers_url":"https://api.github.com/users/kduske/followers","following_url":"https://api.github.com/users/kduske/following{/other_user}","gists_url":"https://api.github.com/users/kduske/gists{/gist_id}","starred_url":"https://api.github.com/users/kduske/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/kduske/subscriptions","organizations_url":"https://api.github.com/users/kduske/orgs","repos_url":"https://api.github.com/users/kduske/repos","events_url":"https://api.github.com/users/kduske/events{/privacy}","received_events_url":"https://api.github.com/users/kduske/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/kduske/TrenchBroom","description":"Cross-Platform Level Editor","fork":false,"url":"https://api.github.com/repos/kduske/TrenchBroom","forks_url":"https://api.github.com/repos/kduske/TrenchBroom/forks","keys_url":"https://api.github.com/repos/kduske/TrenchBroom/keys{/key_id}","collaborators_url":"https://api.github.com/repos/kduske/TrenchBroom/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/kduske/TrenchBroom/teams","hooks_url":"https://api.github.com/repos/kduske/TrenchBroom/hooks","issue_events_url":"https://api.github.com/repos/kduske/TrenchBroom/issues/events{/number}","events_url":"https://api.github.com/repos/kduske/TrenchBroom/events","assignees_url":"https://api.github.com/repos/kduske/TrenchBroom/assignees{/user}","branches_url":"https://api.github.com/repos/kduske/TrenchBroom/branches{/branch}","tags_url":"https://api.github.com/repos/kduske/TrenchBroom/tags","blobs_url":"https://api.github.com/repos/kduske/TrenchBroom/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/kduske/TrenchBroom/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/kduske/TrenchBroom/git/refs{/sha}","trees_url":"https://api.github.com/repos/kduske/TrenchBroom/git/trees{/sha}","statuses_url":"https://api.github.com/repos/kduske/TrenchBroom/statuses/{sha}","languages_url":"https://api.github.com/repos/kduske/TrenchBroom/languages","stargazers_url":"https://api.github.com/repos/kduske/TrenchBroom/stargazers","contributors_url":"https://api.github.com/repos/kduske/TrenchBroom/contributors","subscribers_url":"https://api.github.com/repos/kduske/TrenchBroom/subscribers","subscription_url":"https://api.github.com/repos/kduske/TrenchBroom/subscription","commits_url":"https://api.github.com/repos/kduske/TrenchBroom/commits{/sha}","git_commits_url":"https://api.github.com/repos/kduske/TrenchBroom/git/commits{/sha}","comments_url":"https://api.github.com/repos/kduske/TrenchBroom/comments{/number}","issue_comment_url":"https://api.github.com/repos/kduske/TrenchBroom/issues/comments{/number}","contents_url":"https://api.github.com/repos/kduske/TrenchBroom/contents/{+path}","compare_url":"https://api.github.com/repos/kduske/TrenchBroom/compare/{base}...{head}","merges_url":"https://api.github.com/repos/kduske/TrenchBroom/merges","archive_url":"https://api.github.com/repos/kduske/TrenchBroom/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/kduske/TrenchBroom/downloads","issues_url":"https://api.github.com/repos/kduske/TrenchBroom/issues{/number}","pulls_url":"https://api.github.com/repos/kduske/TrenchBroom/pulls{/number}","milestones_url":"https://api.github.com/repos/kduske/TrenchBroom/milestones{/number}","notifications_url":"https://api.github.com/repos/kduske/TrenchBroom/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/kduske/TrenchBroom/labels{/name}","releases_url":"https://api.github.com/repos/kduske/TrenchBroom/releases{/id}","deployments_url":"https://api.github.com/repos/kduske/TrenchBroom/deployments","created_at":"2012-04-12T09:39:51Z","updated_at":"2018-06-24T23:47:41Z","pushed_at":"2018-06-26T20:47:08Z","git_url":"git://github.com/kduske/TrenchBroom.git","ssh_url":"git@github.com:kduske/TrenchBroom.git","clone_url":"https://github.com/kduske/TrenchBroom.git","svn_url":"https://github.com/kduske/TrenchBroom","homepage":"kristianduske.com/trenchbroom","size":112874,"stargazers_count":352,"watchers_count":352,"language":"C++","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":43,"mirror_url":null,"archived":false,"open_issues_count":186,"license":null,"forks":43,"open_issues":186,"watchers":352,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/kduske/TrenchBroom/pulls/2151"},"html":{"href":"https://github.com/kduske/TrenchBroom/pull/2151"},"issue":{"href":"https://api.github.com/repos/kduske/TrenchBroom/issues/2151"},"comments":{"href":"https://api.github.com/repos/kduske/TrenchBroom/issues/2151/comments"},"review_comments":{"href":"https://api.github.com/repos/kduske/TrenchBroom/pulls/2151/comments"},"review_comment":{"href":"https://api.github.com/repos/kduske/TrenchBroom/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/kduske/TrenchBroom/pulls/2151/commits"},"statuses":{"href":"https://api.github.com/repos/kduske/TrenchBroom/statuses/41e9a345144083935fd39a4ef284466d8acae845"}},"author_association":"COLLABORATOR"}}
{"actor":{"display_login":"kduske"}}