Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/full_texture_atlas into lp:widelands
> @SirVer, what else do you want to introduce before b19? I think your changes > are last big changes before we enter the feature freeze. I have one more smaller refactoring out that I want to get in [1]. I wanted to build zooming into the game (that was, what got me started on the whole GL improvements road), but I agree that this can wait till after build 19, so I delay that. Once these branches are in, I'll focus on getting bugs fixed for b19 and I am fine with feature freezing. There are functional changes that need to happen but which I consider bug fixing - for example updating Eris, our Lua persistent library in third_party/ is a necessity to not break savegames in the future again. [1] https://code.launchpad.net/~widelands-dev/widelands/split_overlay_manager/+merge/254496 -- https://code.launchpad.net/~widelands-dev/widelands/full_texture_atlas/+merge/281909 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/full_texture_atlas into lp:widelands. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Build #8825300] i386 build of widelands 1:18-ppa0-bzr7696-201601091817~ubuntu15.04.1 in ubuntu vivid RELEASE [~widelands-dev/ubuntu/widelands-daily]
* Source Package: widelands * Version: 1:18-ppa0-bzr7696-201601091817~ubuntu15.04.1 * Architecture: i386 * Archive: ~widelands-dev/ubuntu/widelands-daily * Component: main * State: Failed to build * Duration: 21 minutes * Build Log: https://launchpad.net/~widelands-dev/+archive/ubuntu/widelands-daily/+build/8825300/+files/buildlog_ubuntu-vivid-i386.widelands_1%3A18-ppa0-bzr7696-201601091817~ubuntu15.04.1_BUILDING.txt.gz * Builder: https://launchpad.net/builders/lgw01-12 * Source: not available If you want further information about this situation, feel free to contact a member of the Launchpad Buildd Administrators team. -- i386 build of widelands 1:18-ppa0-bzr7696-201601091817~ubuntu15.04.1 in ubuntu vivid RELEASE https://launchpad.net/~widelands-dev/+archive/ubuntu/widelands-daily/+build/8825300 You are receiving this email because you created this version of this package. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Build #8825297] amd64 build of widelands 1:18-ppa0-bzr7696-201601091817~ubuntu14.04.1 in ubuntu trusty RELEASE [~widelands-dev/ubuntu/widelands-daily]
* Source Package: widelands * Version: 1:18-ppa0-bzr7696-201601091817~ubuntu14.04.1 * Architecture: amd64 * Archive: ~widelands-dev/ubuntu/widelands-daily * Component: main * State: Failed to build * Duration: 22 minutes * Build Log: https://launchpad.net/~widelands-dev/+archive/ubuntu/widelands-daily/+build/8825297/+files/buildlog_ubuntu-trusty-amd64.widelands_1%3A18-ppa0-bzr7696-201601091817~ubuntu14.04.1_BUILDING.txt.gz * Builder: https://launchpad.net/builders/lgw01-16 * Source: not available If you want further information about this situation, feel free to contact a member of the Launchpad Buildd Administrators team. -- amd64 build of widelands 1:18-ppa0-bzr7696-201601091817~ubuntu14.04.1 in ubuntu trusty RELEASE https://launchpad.net/~widelands-dev/+archive/ubuntu/widelands-daily/+build/8825297 You are receiving this email because you created this version of this package. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/table_align into lp:widelands
Diff comments:
>
> === modified file 'src/ui_basic/table.cc'
> --- src/ui_basic/table.cc 2015-12-31 08:36:41 +
> +++ src/ui_basic/table.cc 2016-01-06 12:13:29 +
> @@ -365,8 +374,25 @@
> text_width = text_width + m_scrollbar->get_w();
> }
> UI::correct_for_align(alignment, text_width,
> entry_text_im->height(), );
> - // Crop to column width
> - dst.blitrect(point, entry_text_im, Rect(0, 0, curw -
> picw, lineheight));
> +
> + // Crop to column width while blitting
> + if (((static_cast(curw) + picw) < text_width))
> {
What I am suggestion is to always use int when overflows is unlikely. Reason is
that errors with arithmetic between signed and unsigned are way more often than
overflow errors. This is also coded in the Google c++ style guide here. See
here for an explanation
https://google.github.io/styleguide/cppguide.html#Integer_Types , On Unsigned
Types.
> + // Fix positioning for BiDi languages.
> + if (UI::g_fh1->fontset().is_rtl()) {
> + point.x = alignment & Align_Right ?
> curx : curx + picw;
> + }
> + // We want this always on, e.g. for mixed
> language savegame filenames
> + if
> (i18n::has_rtl_character(entry_string.c_str(), 20)) { // Restrict check for
> efficiency
> + dst.blitrect(point,
> +
> entry_text_im,
> +
> Rect(text_width - curw + picw, 0, text_width, lineheight));
> + }
> + else {
> + dst.blitrect(point, entry_text_im,
> Rect(0, 0, curw - picw, lineheight));
> + }
> + } else {
> + dst.blitrect(point, entry_text_im, Rect(0, 0,
> curw - picw, lineheight));
> + }
> curx += curw;
> }
>
--
https://code.launchpad.net/~widelands-dev/widelands/table_align/+merge/279685
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/table_align.
___
Mailing list: https://launchpad.net/~widelands-dev
Post to : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Build #8825305] amd64 build of widelands 1:18-ppa0-bzr7696-201601091817~ubuntu15.10.1 in ubuntu wily RELEASE [~widelands-dev/ubuntu/widelands-daily]
* Source Package: widelands * Version: 1:18-ppa0-bzr7696-201601091817~ubuntu15.10.1 * Architecture: amd64 * Archive: ~widelands-dev/ubuntu/widelands-daily * Component: main * State: Failed to build * Duration: 21 minutes * Build Log: https://launchpad.net/~widelands-dev/+archive/ubuntu/widelands-daily/+build/8825305/+files/buildlog_ubuntu-wily-amd64.widelands_1%3A18-ppa0-bzr7696-201601091817~ubuntu15.10.1_BUILDING.txt.gz * Builder: https://launchpad.net/builders/lgw01-51 * Source: not available If you want further information about this situation, feel free to contact a member of the Launchpad Buildd Administrators team. -- amd64 build of widelands 1:18-ppa0-bzr7696-201601091817~ubuntu15.10.1 in ubuntu wily RELEASE https://launchpad.net/~widelands-dev/+archive/ubuntu/widelands-daily/+build/8825305 You are receiving this email because you created this version of this package. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Build #8825298] i386 build of widelands 1:18-ppa0-bzr7696-201601091817~ubuntu14.04.1 in ubuntu trusty RELEASE [~widelands-dev/ubuntu/widelands-daily]
* Source Package: widelands * Version: 1:18-ppa0-bzr7696-201601091817~ubuntu14.04.1 * Architecture: i386 * Archive: ~widelands-dev/ubuntu/widelands-daily * Component: main * State: Failed to build * Duration: 33 minutes * Build Log: https://launchpad.net/~widelands-dev/+archive/ubuntu/widelands-daily/+build/8825298/+files/buildlog_ubuntu-trusty-i386.widelands_1%3A18-ppa0-bzr7696-201601091817~ubuntu14.04.1_BUILDING.txt.gz * Builder: https://launchpad.net/builders/lgw01-38 * Source: not available If you want further information about this situation, feel free to contact a member of the Launchpad Buildd Administrators team. -- i386 build of widelands 1:18-ppa0-bzr7696-201601091817~ubuntu14.04.1 in ubuntu trusty RELEASE https://launchpad.net/~widelands-dev/+archive/ubuntu/widelands-daily/+build/8825298 You are receiving this email because you created this version of this package. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Build #8825299] amd64 build of widelands 1:18-ppa0-bzr7696-201601091817~ubuntu15.04.1 in ubuntu vivid RELEASE [~widelands-dev/ubuntu/widelands-daily]
* Source Package: widelands * Version: 1:18-ppa0-bzr7696-201601091817~ubuntu15.04.1 * Architecture: amd64 * Archive: ~widelands-dev/ubuntu/widelands-daily * Component: main * State: Failed to build * Duration: 20 minutes * Build Log: https://launchpad.net/~widelands-dev/+archive/ubuntu/widelands-daily/+build/8825299/+files/buildlog_ubuntu-vivid-amd64.widelands_1%3A18-ppa0-bzr7696-201601091817~ubuntu15.04.1_BUILDING.txt.gz * Builder: https://launchpad.net/builders/lcy01-15 * Source: not available If you want further information about this situation, feel free to contact a member of the Launchpad Buildd Administrators team. -- amd64 build of widelands 1:18-ppa0-bzr7696-201601091817~ubuntu15.04.1 in ubuntu vivid RELEASE https://launchpad.net/~widelands-dev/+archive/ubuntu/widelands-daily/+build/8825299 You are receiving this email because you created this version of this package. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Build #8825306] i386 build of widelands 1:18-ppa0-bzr7696-201601091817~ubuntu15.10.1 in ubuntu wily RELEASE [~widelands-dev/ubuntu/widelands-daily]
* Source Package: widelands * Version: 1:18-ppa0-bzr7696-201601091817~ubuntu15.10.1 * Architecture: i386 * Archive: ~widelands-dev/ubuntu/widelands-daily * Component: main * State: Failed to build * Duration: 17 minutes * Build Log: https://launchpad.net/~widelands-dev/+archive/ubuntu/widelands-daily/+build/8825306/+files/buildlog_ubuntu-wily-i386.widelands_1%3A18-ppa0-bzr7696-201601091817~ubuntu15.10.1_BUILDING.txt.gz * Builder: https://launchpad.net/builders/lcy01-09 * Source: not available If you want further information about this situation, feel free to contact a member of the Launchpad Buildd Administrators team. -- i386 build of widelands 1:18-ppa0-bzr7696-201601091817~ubuntu15.10.1 in ubuntu wily RELEASE https://launchpad.net/~widelands-dev/+archive/ubuntu/widelands-daily/+build/8825306 You are receiving this email because you created this version of this package. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Build #8825303] amd64 build of widelands 1:18-ppa0-bzr7696-201601091818~ubuntu16.04.1 in ubuntu xenial RELEASE [~widelands-dev/ubuntu/widelands-daily]
* Source Package: widelands * Version: 1:18-ppa0-bzr7696-201601091818~ubuntu16.04.1 * Architecture: amd64 * Archive: ~widelands-dev/ubuntu/widelands-daily * Component: main * State: Failed to build * Duration: 25 minutes * Build Log: https://launchpad.net/~widelands-dev/+archive/ubuntu/widelands-daily/+build/8825303/+files/buildlog_ubuntu-xenial-amd64.widelands_1%3A18-ppa0-bzr7696-201601091818~ubuntu16.04.1_BUILDING.txt.gz * Builder: https://launchpad.net/builders/lgw01-17 * Source: not available If you want further information about this situation, feel free to contact a member of the Launchpad Buildd Administrators team. -- amd64 build of widelands 1:18-ppa0-bzr7696-201601091818~ubuntu16.04.1 in ubuntu xenial RELEASE https://launchpad.net/~widelands-dev/+archive/ubuntu/widelands-daily/+build/8825303 You are receiving this email because you created this version of this package. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/full_texture_atlas into lp:widelands
I agree that this should be changed now - before build 19. This is performance improvement and game needs it badly. But I also agree that we need to declare feature freeze ASAP and focus on feature freeze. @SirVer, what else do you want to introduce before b19? I think your changes are last big changes before we enter the feature freeze. What do you think? -- https://code.launchpad.net/~widelands-dev/widelands/full_texture_atlas/+merge/281909 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/full_texture_atlas into lp:widelands. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Build #8825304] i386 build of widelands 1:18-ppa0-bzr7696-201601091818~ubuntu16.04.1 in ubuntu xenial RELEASE [~widelands-dev/ubuntu/widelands-daily]
* Source Package: widelands * Version: 1:18-ppa0-bzr7696-201601091818~ubuntu16.04.1 * Architecture: i386 * Archive: ~widelands-dev/ubuntu/widelands-daily * Component: main * State: Failed to build * Duration: 25 minutes * Build Log: https://launchpad.net/~widelands-dev/+archive/ubuntu/widelands-daily/+build/8825304/+files/buildlog_ubuntu-xenial-i386.widelands_1%3A18-ppa0-bzr7696-201601091818~ubuntu16.04.1_BUILDING.txt.gz * Builder: https://launchpad.net/builders/lgw01-13 * Source: not available If you want further information about this situation, feel free to contact a member of the Launchpad Buildd Administrators team. -- i386 build of widelands 1:18-ppa0-bzr7696-201601091818~ubuntu16.04.1 in ubuntu xenial RELEASE https://launchpad.net/~widelands-dev/+archive/ubuntu/widelands-daily/+build/8825304 You are receiving this email because you created this version of this package. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] Bunnybot says...
Hi, I am bunnybot (https://github.com/widelands/bunnybot). I am keeping the source branch lp:~widelands-dev/widelands/use_image_cache mirrored to https://github.com/widelands/widelands/tree/_widelands_dev_widelands_use_image_cache You can give me commands by starting a line with @bunnybot . I understand: merge: Merges the source branch into the target branch, closing the merge proposal. I will use the proposed commit message if it is set. -- https://code.launchpad.net/~widelands-dev/widelands/use_image_cache/+merge/282106 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/use_image_cache into lp:widelands. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/use_image_cache into lp:widelands
This is an impressive improvement! Out of curiosity, how large does the texture
atlas end up being?
I have mostly looked at the actual drawing code (I did notice a merge conflict
marker in world.cc though, and bunnybot probably did, too ;)). As I already
said today on IRC, for desktop OpenGL you should really use instancing. You can
copy the Arguments structure almost 1:1 into a buffer with per-instance data
and use a simple vertex shader to reduce the CPU load. I'd understand if you
don't want separate paths for OpenGL ES 2.0 though.
The next-best thing I noticed is using glMapBuffer to avoid an unnecessary
copy. Right now you fill the vertices_ vector (first "copy"). Then glBufferData
copies the vertex data into the buffer object (second copy). And finally the
vertex data gets uploaded to/pulled by the GPU.
Since you know the number of vertices up-front, it would be better to use
glBufferData(..., NULL, ...) to reserve the required amount of space, then use
glMapBuffer and write the vertex data directly into the buffer object, thus
avoiding the second copy. (This would also apply to
(And now that I've written this, I notice that GLES 2.0 apparently doesn't have
glMapBuffer... *sigh*)
Third, it would almost certainly be beneficial to use an element array. That
would reduce the amount of vertex data you need to write by one third, and
would also reduce the vertex shader load on the GPU by up to one third
(Widelands almost certainly isn't shader bound, but hey... perhaps it saves
some power). The element array can even be completely static - it just needs to
grow as needed during application warm-up.
For the buffer handling, the following is actually best for the kind of
streaming draw that Widelands is doing:
1. Determine a reasonably large buffer size N.
2. Use a single buffer object, initialized with glBufferData(..., N, NULL,
GL_STREAM_DRAW);
3. As you go along drawing a frame, fill the buffer object from front to back.
For all vertex data that you write, use glMapBufferRange to map _exactly_ the
range that you want to write for the next draw call, then unmap, then do the
draw call.
4. When the vertex data for the next draw call wouldn't fit in the remaining
space of the buffer, start from the front, but (and this is VERY important,
I've seen plenty of applications screw this up) add the
GL_MAP_INVALIDATE_BUFFER_BIT.
What happens when you do this is that the driver magically maintains a pool of
underlying buffers, all of size N. Whenever you start from 0 with the
MAP_INVALIDATE_BUFFER_BIT, the driver switches the underlying buffer to one
from the pool that is no longer in-flight. As far as I know, all desktop GL
drivers do this.
Now you see that the patch I sent isn't actually optimal. It would be even
better to do something like this:
GLsizeiptr bytes = items.size() * sizeof(T);
if (bytes > size_) {
glBufferData(GL_ARRAY_BUFFER, bytes, items.data(), GL_STREAM_DRAW);
size_ = bytes;
filled_ = bytes;
} else {
GLbitfield access = GL_MAP_WRITE_BIT;
if (filled_ + bytes > size_) {
filled_ = 0;
access |= GL_MAP_INVALIDATE_BUFFER_BIT;
}
void *map = glMapBufferRange(GL_ARRAY_BUFFER, filled_, bytes, access);
memcpy(map, items.data(), bytes);
glUnmapBuffer(GL_ARRAY_BUFFER);
filled_ = (filled_ + bytes + 3) & ~3; // for the memcpy a larger alignment
might be better; the GPU doesn't really care that much as long as you're dword
aligned
}
This is better because then the underlying buffer is always the same size and
the re-use mechanism can work more efficiently. But then you need to check for
support of GL_ARB_map_buffer_range, and apparently GLES 2.0 doesn't have it.
Also, buffers don't have to be the exact same size to be re-used by the driver,
so the impact is probably not too bad.
And of course, while all of the above should reduce CPU load, I have no idea by
how much.
--
https://code.launchpad.net/~widelands-dev/widelands/use_image_cache/+merge/282106
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/use_image_cache into lp:widelands.
___
Mailing list: https://launchpad.net/~widelands-dev
Post to : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/full_texture_atlas into lp:widelands
I had another idea regarding the loading screen: ui_fsmenu doesn't really need a performance enhancement, so we could not use the texture atlas in the fullscreen menus and generate it in the background, then finish loading it in the game/editor loading screens if necessary. I don't know the consequences for bug hunting/fixing of this though. Another argument in favor of solution 1: modders can change images more easily. The texture atlas could also have a hash on file number/sizes/dates (maybe remember the file date on the newest file for each folder only?), so if a graphic dev changes something, the texture atlas is recalculated. Graphics devs will still need to wait, but they won't need to manually nuke the cache every time. -- https://code.launchpad.net/~widelands-dev/widelands/full_texture_atlas/+merge/281909 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/full_texture_atlas into lp:widelands. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/full_texture_atlas into lp:widelands
Another question: I use an extra datadir folder which contains some special images and that i use for testing purposes. What happens to the --datadir option when this get merged? Could i further use it for testing images? -- https://code.launchpad.net/~widelands-dev/widelands/full_texture_atlas/+merge/281909 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/full_texture_atlas into lp:widelands. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
