Merge lp://staging/~widelands-dev/widelands/bug1098263 into lp://staging/widelands

Proposed by Jens Beyer
Status: Merged
Merged at revision: 6623
Proposed branch: lp://staging/~widelands-dev/widelands/bug1098263
Merge into: lp://staging/widelands
Diff against target: 124 lines (+42/-20) (has conflicts)
3 files modified
src/graphic/graphic.cc (+37/-4)
src/graphic/render/gl_surface_texture.cc (+4/-15)
src/graphic/render/gl_surface_texture.h (+1/-1)
Text conflict in src/graphic/graphic.cc
To merge this branch: bzr merge lp://staging/~widelands-dev/widelands/bug1098263
Reviewer Review Type Date Requested Status
SirVer Approve
Review via email: mp+174275@code.staging.launchpad.net

Description of the change

Ok, I tried to fix this.

The reason I had to split the init function is that the GL_EXTENSIONS seems to be available only late in the game (it looks like glewInit() does provide it).

Problem now is that it looks like we are leaking a sdlsurface instance there (check the TODO), or not? I am not sure.
And I am not sure if I found all possible paths in the code.

Please review thoroughly ;-)

To post a comment you must log in.
Revision history for this message
SirVer (sirver) wrote :

Thanks for looking into this. I spent too much time on Widelands already today, so I'll postpone this review till next week.

And another comment: could you push your branches to ~widelands-dev in the future? that way others can directly edit your branch which makes talking about nits unnecessary.

Revision history for this message
Jens Beyer (qcumber-some) wrote :

I moved this branch to ~widelands-dev and will do so in the future :-)

Revision history for this message
SirVer (sirver) wrote :

I reviewed this in r6603. I changed some nits (only formatting really) and changed the TODO to an explanation. I also deleted some dead code that could never be reached - can you look at my changes if they look fine to you and if they are go ahead and merge this in?

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to status/vote changes: