Update color_picker and implement _draw_polygon. Fixes #8372 #8425
Conversation
|
@MarianoGnu Your changes don't follow the style enforced by clang-format, see https://travis-ci.org/godotengine/godot/jobs/222601404 |
|
|
||
| storage->frame.canvas_draw_commands++; | ||
|
|
||
| #if 0 |
There was a problem hiding this comment.
You're completely changing the implementation of _draw_polygon, is the code below obsolete? What about the WebGL specific stuff?
There was a problem hiding this comment.
the bellow code is from GLES2, i have no idea if it works on WebGL, but if draw_primitive works on webgl this should too, since i used it as base to write it.
I expected to give a fast solution and let someone specialized on renderer to improve it latter.
I have not idea of how clang format's the code...
|
@reduz ping ? |
|
Sorry I'm a quarter planet away from my computer
…On May 7, 2017 7:25 PM, "Mariano Javier Suligoy" ***@***.***> wrote:
@reduz <https://github.com/reduz> ping ?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#8425 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AF-Z29IF5MdIXDRjsqLIEwOV8T5YNIENks5r3gyigaJpZM4M-sAD>
.
|
| stride += 2; | ||
| } | ||
|
|
||
| float b[(2 + 2 + 4) * p_vertex_count]; |
There was a problem hiding this comment.
Variable Length Arrays are not allowed in C++.
Use
float* b = new float[(2 + 2 + 4) * p_vertex_count]();
I'm investigating if we can limit the memory allocation in this method.
There was a problem hiding this comment.
you can't unless you limit the number of vertexes in polygons. this function is also used bt polygon2d nodes and collisionpolygon2d editor helper, so you cant be sure how many vertexes there are going to be. By the other hand, i didn't have any errors compiling, that's weird
There was a problem hiding this comment.
GCC will grandfather VLA from c99 into c++, may be why it compiled?
There was a problem hiding this comment.
Well, we should probably use a RealPoolArray in this case, to "stay godot" 😃
There was a problem hiding this comment.
i think using raw arrays is faster to copy to gpu memory since memory allocation is fixed, but i'm not expert in low level...
There was a problem hiding this comment.
Both options result in the same memory layout, there is no speed difference between the two when copying to the GPU. The end result is the same. PoolRealArray is just the "godot" way.
|
this would be really nice to get merged. |
i think its pending for a review from @reduz |
|
@akien-mga do you have some article so i can figure out what's the clang-format and why the code doesn't fit so i can fix that? |
|
there is a clang .clang-format in the git project. you install clang format on your computer. and than you run |
|
The |
|
i'll close this PR and open a new one, since color_picker.cpp has some merge conflics from another commit. I hope i can do this in the next days, but the sources are at home and i have not internet there yet :( |
http://i.imgur.com/D0EXxN9.png
http://i.imgur.com/vY08sK7.png