Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Merge gl::ObjectStore and gl::Config #6470

Merged
merged 3 commits into from
Sep 27, 2016

Conversation

kkaefer
Copy link
Member

@kkaefer kkaefer commented Sep 27, 2016

These are currently two distinct objects, but they're almost always used together. Passing multiple objects around just makes the code longer and less readable. I'm not seeing any gains from keeping them separated.

@kkaefer kkaefer added the Core The cross-platform C++ core, aka mbgl label Sep 27, 2016
@jfirebaugh jfirebaugh mentioned this pull request Sep 27, 2016
1 task
@mention-bot
Copy link

@kkaefer, thanks for your PR! By analyzing this pull request, we identified @jfirebaugh, @tmpsantos and @ansis to be potential reviewers.

@kkaefer
Copy link
Member Author

kkaefer commented Sep 27, 2016

There are a few tasks that aren't necessarily part of this PR:

  • move gl::debugging into gl::Context
  • hide gl* calls in implementation files
  • cleanup gl.hpp and gl.cpp and move various functions to their own headers

Copy link
Contributor

@jfirebaugh jfirebaugh left a comment

Choose a reason for hiding this comment

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

Looks great, gl::Context is exactly what #6468 needs to build upon.

CI failures look like minor include-what-you-use issues.

@jfirebaugh
Copy link
Contributor

Oh, didn't look at Qt5 macOS failure, that's something more serious:

[ RUN      ] Annotations.UpdateSymbolAnnotationIcon
mbgl-test(11337,0x7fff7acf4000) malloc: *** error for object 0x7f9a1bcc8180: incorrect checksum for freed object - object was probably modified after being freed.
*** set a breakpoint in malloc_error_break to debug
make: *** [run-qt-test-*] Abort trap: 6

@kkaefer kkaefer force-pushed the 6470-merge-gl-config-and-objectstore branch from 251df43 to 1f6c9e0 Compare September 27, 2016 16:31
@kkaefer
Copy link
Member Author

kkaefer commented Sep 27, 2016

Yeah, I think we were destroying the Context before the AnnotationManager.

@jfirebaugh jfirebaugh merged commit 44c7e9d into master Sep 27, 2016
@jfirebaugh jfirebaugh deleted the 6470-merge-gl-config-and-objectstore branch September 27, 2016 18:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Core The cross-platform C++ core, aka mbgl
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants