Skip to content

S
swiftshader
Commit ae7756e0 by Nicolas Capens Committed by Nicolas Capens
Options

Fix assert while updating surface buffers.

When the external buffer of a surface is dirty, and we're trying to lock the internal buffer (or vice-versa), an 'update' needs to happen. This resulted in us locking the internal buffer for writing, thereby marking it as dirty. This triggered an assert which checks that both buffers can't be dirty at the same time. Also, in release mode this could result in redundant updates when the external buffer is locked again. We should mark the destination buffer dirty when we're about to make sure it's up to date, so a new lock type was added which is equivalent to a write lock but without setting the dirty flag. Also, we were allocating memory for a 0x0 stencil buffer for each render target, when there's no stencil component. So return nullptr when the format is NULL. Change-Id: Ie7b5528e3eedc3c3efdf8461047e6284b7bdfc84 Reviewed-on: https://swiftshader-review.googlesource.com/16828Tested-by: 's avatarNicolas Capens <nicolascapens@google.com> Reviewed-by: 's avatarAlexis Hétu <sugoi@google.com>
parent 680bcf50
Showing with 21 additions and 14 deletions
src/Renderer/Surface.cpp
...@@ -1118,6 +1118,7 @@ namespace sw ...@@ -1118,6 +1118,7 @@ namespace sw
{ {
case LOCK_UNLOCKED: case LOCK_UNLOCKED:
case LOCK_READONLY: case LOCK_READONLY:
case LOCK_UPDATE:
break; break;
case LOCK_WRITEONLY: case LOCK_WRITEONLY:
case LOCK_READWRITE: case LOCK_READWRITE:
...@@ -1514,6 +1515,11 @@ namespace sw ...@@ -1514,6 +1515,11 @@ namespace sw
void *Surface::lockStencil(int x, int y, int front, Accessor client) void *Surface::lockStencil(int x, int y, int front, Accessor client)
{ {
if(stencil.format == FORMAT_NULL)
{
return nullptr;
}
resource->lock(client); resource->lock(client);
if(!stencil.buffer) if(!stencil.buffer)
...@@ -1922,7 +1928,7 @@ namespace sw ...@@ -1922,7 +1928,7 @@ namespace sw
void Surface::genericUpdate(Buffer &destination, Buffer &source) void Surface::genericUpdate(Buffer &destination, Buffer &source)
{ {
unsigned char *sourceSlice = (unsigned char*)source.lockRect(0, 0, 0, sw::LOCK_READONLY); unsigned char *sourceSlice = (unsigned char*)source.lockRect(0, 0, 0, sw::LOCK_READONLY);
unsigned char *destinationSlice = (unsigned char*)destination.lockRect(0, 0, 0, sw::LOCK_WRITEONLY); unsigned char *destinationSlice = (unsigned char*)destination.lockRect(0, 0, 0, sw::LOCK_UPDATE);
int depth = min(destination.depth, source.depth); int depth = min(destination.depth, source.depth);
int height = min(destination.height, source.height); int height = min(destination.height, source.height);
...@@ -1970,7 +1976,7 @@ namespace sw ...@@ -1970,7 +1976,7 @@ namespace sw
void Surface::decodeR8G8B8(Buffer &destination, Buffer &source) void Surface::decodeR8G8B8(Buffer &destination, Buffer &source)
{ {
unsigned char *sourceSlice = (unsigned char*)source.lockRect(0, 0, 0, sw::LOCK_READONLY); unsigned char *sourceSlice = (unsigned char*)source.lockRect(0, 0, 0, sw::LOCK_READONLY);
unsigned char *destinationSlice = (unsigned char*)destination.lockRect(0, 0, 0, sw::LOCK_WRITEONLY); unsigned char *destinationSlice = (unsigned char*)destination.lockRect(0, 0, 0, sw::LOCK_UPDATE);
int depth = min(destination.depth, source.depth); int depth = min(destination.depth, source.depth);
int height = min(destination.height, source.height); int height = min(destination.height, source.height);
...@@ -2013,7 +2019,7 @@ namespace sw ...@@ -2013,7 +2019,7 @@ namespace sw
void Surface::decodeX1R5G5B5(Buffer &destination, Buffer &source) void Surface::decodeX1R5G5B5(Buffer &destination, Buffer &source)
{ {
unsigned char *sourceSlice = (unsigned char*)source.lockRect(0, 0, 0, sw::LOCK_READONLY); unsigned char *sourceSlice = (unsigned char*)source.lockRect(0, 0, 0, sw::LOCK_READONLY);
unsigned char *destinationSlice = (unsigned char*)destination.lockRect(0, 0, 0, sw::LOCK_WRITEONLY); unsigned char *destinationSlice = (unsigned char*)destination.lockRect(0, 0, 0, sw::LOCK_UPDATE);
int depth = min(destination.depth, source.depth); int depth = min(destination.depth, source.depth);
int height = min(destination.height, source.height); int height = min(destination.height, source.height);
...@@ -2058,7 +2064,7 @@ namespace sw ...@@ -2058,7 +2064,7 @@ namespace sw
void Surface::decodeA1R5G5B5(Buffer &destination, Buffer &source) void Surface::decodeA1R5G5B5(Buffer &destination, Buffer &source)
{ {
unsigned char *sourceSlice = (unsigned char*)source.lockRect(0, 0, 0, sw::LOCK_READONLY); unsigned char *sourceSlice = (unsigned char*)source.lockRect(0, 0, 0, sw::LOCK_READONLY);
unsigned char *destinationSlice = (unsigned char*)destination.lockRect(0, 0, 0, sw::LOCK_WRITEONLY); unsigned char *destinationSlice = (unsigned char*)destination.lockRect(0, 0, 0, sw::LOCK_UPDATE);
int depth = min(destination.depth, source.depth); int depth = min(destination.depth, source.depth);
int height = min(destination.height, source.height); int height = min(destination.height, source.height);
...@@ -2104,7 +2110,7 @@ namespace sw ...@@ -2104,7 +2110,7 @@ namespace sw
void Surface::decodeX4R4G4B4(Buffer &destination, Buffer &source) void Surface::decodeX4R4G4B4(Buffer &destination, Buffer &source)
{ {
unsigned char *sourceSlice = (unsigned char*)source.lockRect(0, 0, 0, sw::LOCK_READONLY); unsigned char *sourceSlice = (unsigned char*)source.lockRect(0, 0, 0, sw::LOCK_READONLY);
unsigned char *destinationSlice = (unsigned char*)destination.lockRect(0, 0, 0, sw::LOCK_WRITEONLY); unsigned char *destinationSlice = (unsigned char*)destination.lockRect(0, 0, 0, sw::LOCK_UPDATE);
int depth = min(destination.depth, source.depth); int depth = min(destination.depth, source.depth);
int height = min(destination.height, source.height); int height = min(destination.height, source.height);
...@@ -2149,7 +2155,7 @@ namespace sw ...@@ -2149,7 +2155,7 @@ namespace sw
void Surface::decodeA4R4G4B4(Buffer &destination, Buffer &source) void Surface::decodeA4R4G4B4(Buffer &destination, Buffer &source)
{ {
unsigned char *sourceSlice = (unsigned char*)source.lockRect(0, 0, 0, sw::LOCK_READONLY); unsigned char *sourceSlice = (unsigned char*)source.lockRect(0, 0, 0, sw::LOCK_READONLY);
unsigned char *destinationSlice = (unsigned char*)destination.lockRect(0, 0, 0, sw::LOCK_WRITEONLY); unsigned char *destinationSlice = (unsigned char*)destination.lockRect(0, 0, 0, sw::LOCK_UPDATE);
int depth = min(destination.depth, source.depth); int depth = min(destination.depth, source.depth);
int height = min(destination.height, source.height); int height = min(destination.height, source.height);
...@@ -2195,7 +2201,7 @@ namespace sw ...@@ -2195,7 +2201,7 @@ namespace sw
void Surface::decodeP8(Buffer &destination, Buffer &source) void Surface::decodeP8(Buffer &destination, Buffer &source)
{ {
unsigned char *sourceSlice = (unsigned char*)source.lockRect(0, 0, 0, sw::LOCK_READONLY); unsigned char *sourceSlice = (unsigned char*)source.lockRect(0, 0, 0, sw::LOCK_READONLY);
unsigned char *destinationSlice = (unsigned char*)destination.lockRect(0, 0, 0, sw::LOCK_WRITEONLY); unsigned char *destinationSlice = (unsigned char*)destination.lockRect(0, 0, 0, sw::LOCK_UPDATE);
int depth = min(destination.depth, source.depth); int depth = min(destination.depth, source.depth);
int height = min(destination.height, source.height); int height = min(destination.height, source.height);
...@@ -2240,7 +2246,7 @@ namespace sw ...@@ -2240,7 +2246,7 @@ namespace sw
void Surface::decodeDXT1(Buffer &internal, Buffer &external) void Surface::decodeDXT1(Buffer &internal, Buffer &external)
{ {
unsigned int *destSlice = (unsigned int*)internal.lockRect(0, 0, 0, LOCK_WRITEONLY); unsigned int *destSlice = (unsigned int*)internal.lockRect(0, 0, 0, LOCK_UPDATE);
const DXT1 *source = (const DXT1*)external.lockRect(0, 0, 0, LOCK_READONLY); const DXT1 *source = (const DXT1*)external.lockRect(0, 0, 0, LOCK_READONLY);
for(int z = 0; z < external.depth; z++) for(int z = 0; z < external.depth; z++)
...@@ -2305,7 +2311,7 @@ namespace sw ...@@ -2305,7 +2311,7 @@ namespace sw
void Surface::decodeDXT3(Buffer &internal, Buffer &external) void Surface::decodeDXT3(Buffer &internal, Buffer &external)
{ {
unsigned int *destSlice = (unsigned int*)internal.lockRect(0, 0, 0, LOCK_WRITEONLY); unsigned int *destSlice = (unsigned int*)internal.lockRect(0, 0, 0, LOCK_UPDATE);
const DXT3 *source = (const DXT3*)external.lockRect(0, 0, 0, LOCK_READONLY); const DXT3 *source = (const DXT3*)external.lockRect(0, 0, 0, LOCK_READONLY);
for(int z = 0; z < external.depth; z++) for(int z = 0; z < external.depth; z++)
...@@ -2355,7 +2361,7 @@ namespace sw ...@@ -2355,7 +2361,7 @@ namespace sw
void Surface::decodeDXT5(Buffer &internal, Buffer &external) void Surface::decodeDXT5(Buffer &internal, Buffer &external)
{ {
unsigned int *destSlice = (unsigned int*)internal.lockRect(0, 0, 0, LOCK_WRITEONLY); unsigned int *destSlice = (unsigned int*)internal.lockRect(0, 0, 0, LOCK_UPDATE);
const DXT5 *source = (const DXT5*)external.lockRect(0, 0, 0, LOCK_READONLY); const DXT5 *source = (const DXT5*)external.lockRect(0, 0, 0, LOCK_READONLY);
for(int z = 0; z < external.depth; z++) for(int z = 0; z < external.depth; z++)
...@@ -2429,7 +2435,7 @@ namespace sw ...@@ -2429,7 +2435,7 @@ namespace sw
void Surface::decodeATI1(Buffer &internal, Buffer &external) void Surface::decodeATI1(Buffer &internal, Buffer &external)
{ {
byte *destSlice = (byte*)internal.lockRect(0, 0, 0, LOCK_WRITEONLY); byte *destSlice = (byte*)internal.lockRect(0, 0, 0, LOCK_UPDATE);
const ATI1 *source = (const ATI1*)external.lockRect(0, 0, 0, LOCK_READONLY); const ATI1 *source = (const ATI1*)external.lockRect(0, 0, 0, LOCK_READONLY);
for(int z = 0; z < external.depth; z++) for(int z = 0; z < external.depth; z++)
...@@ -2485,7 +2491,7 @@ namespace sw ...@@ -2485,7 +2491,7 @@ namespace sw
void Surface::decodeATI2(Buffer &internal, Buffer &external) void Surface::decodeATI2(Buffer &internal, Buffer &external)
{ {
word *destSlice = (word*)internal.lockRect(0, 0, 0, LOCK_WRITEONLY); word *destSlice = (word*)internal.lockRect(0, 0, 0, LOCK_UPDATE);
const ATI2 *source = (const ATI2*)external.lockRect(0, 0, 0, LOCK_READONLY); const ATI2 *source = (const ATI2*)external.lockRect(0, 0, 0, LOCK_READONLY);
for(int z = 0; z < external.depth; z++) for(int z = 0; z < external.depth; z++)
...@@ -2568,7 +2574,7 @@ namespace sw ...@@ -2568,7 +2574,7 @@ namespace sw
void Surface::decodeETC2(Buffer &internal, Buffer &external, int nbAlphaBits, bool isSRGB) void Surface::decodeETC2(Buffer &internal, Buffer &external, int nbAlphaBits, bool isSRGB)
{ {
ETC_Decoder::Decode((const byte*)external.lockRect(0, 0, 0, LOCK_READONLY), (byte*)internal.lockRect(0, 0, 0, LOCK_WRITEONLY), external.width, external.height, internal.width, internal.height, internal.pitchB, internal.bytes, ETC_Decoder::Decode((const byte*)external.lockRect(0, 0, 0, LOCK_READONLY), (byte*)internal.lockRect(0, 0, 0, LOCK_UPDATE), external.width, external.height, internal.width, internal.height, internal.pitchB, internal.bytes,
(nbAlphaBits == 8) ? ETC_Decoder::ETC_RGBA : ((nbAlphaBits == 1) ? ETC_Decoder::ETC_RGB_PUNCHTHROUGH_ALPHA : ETC_Decoder::ETC_RGB)); (nbAlphaBits == 8) ? ETC_Decoder::ETC_RGBA : ((nbAlphaBits == 1) ? ETC_Decoder::ETC_RGB_PUNCHTHROUGH_ALPHA : ETC_Decoder::ETC_RGB));
external.unlockRect(); external.unlockRect();
internal.unlockRect(); internal.unlockRect();
......
src/Renderer/Surface.hpp
...@@ -233,7 +233,8 @@ namespace sw ...@@ -233,7 +233,8 @@ namespace sw
LOCK_READONLY, LOCK_READONLY,
LOCK_WRITEONLY, LOCK_WRITEONLY,
LOCK_READWRITE, LOCK_READWRITE,
LOCK_DISCARD LOCK_DISCARD,
LOCK_UPDATE // Write access which doesn't dirty the buffer, because it's being updated with the sibling's data.
}; };
class [[clang::lto_visibility_public]] Surface class [[clang::lto_visibility_public]] Surface
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment