#89 network rendering bug

open-accepted
nobody
None
5
2008-04-21
2008-04-16
No

Hi,

There's a bug in the rcSend() function in rendererNetwork.cpp, buffer is overwritten by a htonl() function if "toNetwork" parameter is not zero.

I've added temp buffer to avoid this:

void rcSend(SOCKET s,const void *dataToSend,int n,int toNetwork) {
int i,j;
const char *data = NULL;
T32 *buf = NULL;

if (toNetwork) {
buf = (T32*)malloc(n);

if (!buf)
return;

memcpy ( buf, dataToSend, n);

T32* buffer = buf;

for (i=n>>2;i>0;i--,buffer++) {
buffer->integer = htonl(buffer->integer);
}

data = (const char *) buf;
} else{
data = (const char *) dataToSend;
}
j = n;
i = send(s,data,j,0);

if (i <= 0) {
fatal(CODE_SYSTEM,"Connection broken\n");
}

// If we could not send the entire data, send the rest
while(i < j) {
data += i;
j -= i;

i = send(s,data,j,0);

if (i <= 0) {
fatal(CODE_SYSTEM,"Connection broken\n");
break;
}
}

if (buf)
free((void*)data);

stats.totalNetSend += n;
}

Discussion

  • Cedric PAILLE

    Cedric PAILLE - 2008-04-16

    Logged In: YES
    user_id=954786
    Originator: YES

    Oh, forgot to say that it was blocking the deepshadow network-rendering.

    Cheers.

     
  • Nobody/Anonymous

    Logged In: NO

    The original code works well on Unix.

     
  • Okan Arikan

    Okan Arikan - 2008-04-21

    Logged In: YES
    user_id=555300
    Originator: NO

    This is an interesting solution, but I'm hesitant to add it into the trunk because it may use malloc unnecessarily. Most of the code assumes the data that is sent over the network can be modified.

    I isolated the piece of code that does not respect this assumption. This should fix the issue with the deep shadow map rendering problem over the network.

    Cedric, may I ask you to checkout the SVN trunk and see if this problem is still present?

    PS: I'm leaving this bug open until I verify that I fixed it.

    Thank you,

    Okan

     
  • Okan Arikan

    Okan Arikan - 2008-04-21
    • status: open --> open-accepted
     
  • Cedric PAILLE

    Cedric PAILLE - 2008-04-22

    Logged In: YES
    user_id=954786
    Originator: YES

    Hi,

    Yes I've updated the svn.

    The problem I encountered is in this function, look at the variable "sz":

    int CRemoteTSMChannel::sendRemoteBucket(SOCKET s,int x,int y) {
    // record current position, seek back to tile start
    uint64_t curPos = ftell(tsmFile);
    fseek(tsmFile,lastPosition,SEEK_SET);
    uint64_t sz = curPos - lastPosition;
    rcSend(s,&sz,sizeof(uint64_t));

    // send the tile data
    char buf[NETWORK_BUFFER_LENGTH];
    while(sz > 0){
    int nn = (int) ((sz>(NETWORK_BUFFER_LENGTH)) ? (NETWORK_BUFFER_LENGTH) : sz);
    fread(buf,nn,1,tsmFile);
    rcSend(s,buf,nn,FALSE);
    sz -= nn;
    }

    "sz" is byte-swapped (on little-endian machines) and is not valid anymore for the while loop.

    Cheers.

     

Log in to post a comment.

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:

JavaScript is required for this form.





No, thanks