These are benign but worth fixing as it clears the log to find more important errors.
A common problem with the sanitizer is that enums are often used to represent bits (e.g. 1, 2, 4, 8 etc) but without specifying the enum type, the compiler is free to use unsigned or signed int. In this case it uses int, and when it performs bitwise operations on the int type, the sanitizer complains.
This is probably because a bitshift with negative signed value can give undefined behaviour - the sanitizer can't know ahead of time that you are using the enum for sensible bitflags.
Valgrind reported two instances of reading uninitialized memory in the batching. They are both pretty benign (as evidenced by no bug reports) but wise to close these.
The first is that when changing batch from a default batch it reads the batch color which is not set (as it is not relevant for default batches). The segment of code is not necessary when it has already deemed a batch change necessary (which will occur from a default batch). In addition this means that the count of color changes will be more accurate, rather than having a possible random value in.
The second is that on initialization _set_texture_rect_mode is called before the state has been properly initialized (it is initialized at the beginning of each canvas_begin, but this occurs outside of that).
Reordering an item from after a copybackbuffer to before would result in the wrong thing being rendered into the backbuffer.
This PR puts in a check to prevent reordering over such a boundary.
Happy new year to the wonderful Godot community!
2020 has been a tough year for most of us personally, but a good year for
Godot development nonetheless with a huge amount of work done towards Godot
4.0 and great improvements backported to the long-lived 3.2 branch.
We've had close to 400 contributors to engine code this year, authoring near
7,000 commit! (And that's only for the `master` branch and for the engine code,
there's a lot more when counting docs, demos and other first-party repos.)
Here's to a great year 2021 for all Godot users 🎆
(cherry picked from commit b5334d14f7)
Ninepatch code has a check to prevent use of zero sized textures. This didn't deal properly with animated textures, which use a proxy (link to another texture).
This PR uses a generalised method of getting textures, with built in support for proxy textures and protection against infinite loops.
These were only put in for the betas, in order to test hypotheses for stalling on Macs. It seems that most of the problems in the Mac editor have been solved by fixing the excessive redraw_requests.
As a result no one has reported any results from these options, but in future we will be able to refer users to try the beta versions, so there is no need to include them in the stable release. Indeed they are only likely to cause confusion.
Although the minimum size of ninepatches is set to the sum of the margins in normal use (through gdscript etc) it turns out that it is possible to programmatically create ninepatches that are small than this minimum - in particular zero size is used in sliders to not draw items.
This PR deals with zero sized ninepatches by not drawing anything, and has some basic protection for ninepatches smaller than the margins. Whether these occur in the wild is not clear but is put in for completeness.
For fixing a previous issue state.canvas_texscreen_used was reset to false at the start of each render_joined_item. This was causing a later shader that used SCREEN_TEXTURE to force recapturing the back buffer immediately prior to use, which we don't want.
This PR preserves the state across joined items, and also prevents joining of items that copy the back buffer as this may be problematic.
It turns out that the original issue that needed the line is now fixed, and the later issue is also fixed by removing it.
While adding more debug checks to legacy renderer, I closed 2 types of vulnerabilities:
* TYPE_PRIMITIVE would previously read from uninitialized data if only specifying a single color
* Other legacy draw operations would fail in debug AFTER accessing out of bounds memory rather than before
Many calls to glBufferSubData are wrapped in a safe version which checks for out of bounds and exits the draw function if this is detected.
Large FVF allows batching of many custom shaders, but should not join items which have shaders that utilize BUILTINs which would change for each item, because these will not be sent individually, and all joined items would wrongly use the values from the first joined item.
Polys that have no texture assigned contain no UVs in the poly command. These were previously not blanked, leading to random values if read from a custom shader.
This PR just blanks them.
In some situations where polygons were scaled, existing software skinning was producing incorrect results.
The transform inverse needed to use an affine inverse rather than a cheaper inverse to account for this scaling.
This adds support for custom shaders for polys, and properly handles modulate in the case of large FVF and modulate FVF.
It also fixes poly vertex colors not being sent to OpenGL.
Antialiased polys work by drawing a smoothed line around the poly after the main drawing. Batching draws polys as a series of triangles with no concept of 'edge', and when 2 polys are joined it becomes impractical to back calculate the edges from the triangles.
For this reason batching is disabled for antialiased polys in this PR.
As a result of the GLES specifications being vague about best practice for how buffers should be used dynamically, different GPUs / platforms appear to have different preferences.
Mac in particular seems to have a number of problems in this area, and none of the rendering team uses Macs. So far we have relied on guesswork to choose the best usage, but in an attempt to pin this down, this PR begins to introduce manual selection of options for users to test their configurations.
Lines are batched using the simplest fvf 'BatchVertex', however when used in an item with a custom shader material, it may attempt to translate to large_fvf without the required extra channels. To prevent this a special case in flushing is made to deal with lines.
In small batches using hardware transform, vertices would be drawn in incorrect positions due to the item transform being applied twice - once in the transform uniform, and once from the transform passed as a vertex attribute.
This PR alters the shader to ignore uniform transforms when using large FVF.
Batching is mostly separated into a common template which can be used with multiple backends (GLES2 and GLES3 here). Only necessary specifics are in the backend files.
Batching is extended to cover more primitives.
lawnjelly
Rémi Verschelde