ellson / MOTHBALLED-graphviz

Moved to https://gitlab.com/graphviz/graphviz
Eclipse Public License 1.0
1.29k stars 255 forks source link

Unexpected artifacts in png output after deleting node #1199

Open chtenb opened 7 years ago

chtenb commented 7 years ago

Consider the following bit of test code.

void dump(Agraph_t* root, string filename)
{
    GVC_t* gvc = gvContext();
    imagwrite(root, (filename + ".dot").c_str());
    gvLayout(gvc, root, "dot");
    gvRenderFilename(gvc, root, "png", (filename + ".png").c_str());
}

void imdebug()
{
    Agraph_t* root = agopen("test root graph", Agdirected, &memDisc);

    Agnode_t* n1 = agnode(root, "x", 1);
    Agnode_t* n2 = agnode(root, "xx", 1);
    Agnode_t* n3 = agnode(root, "xxx", 1);
    Agnode_t* n4 = agnode(root, "xxxx", 1);

    agedge(root, n2, n4, "1", 1);
    agedge(root, n3, n4, "1", 1);
    agedge(root, n1, n2, "1", 1);
    agedge(root, n2, n1, "2", 1);
    agedge(root, n1, n3, "1", 1);
    agedge(root, n3, n1, "2", 1);
    agedge(root, n2, n3, "1", 1);
    agedge(root, n3, n2, "2", 1);

    dump(root, "C:\\Users\\chiel.tenbrinke\\Desktop\\dump1");
    agdelete(root, n1);
    dump(root, "C:\\Users\\chiel.tenbrinke\\Desktop\\dump2");
}

dump1.png: dump1

dump2.png: dump2

As you see, there are two seemingly stale edges partially visible in dump2.png that I wouldn't expect there. The contents of the dot dumps is as follows.

dump1.dot:

digraph "test root graph" {
    graph [bb="0 0 109 252"];
    node [label="\N"];
    xx -> xxx [key=1];
xx -> xxxx [key=1];
xxx -> xx [key=2];
xxx -> xxxx [key=1];
}

dump2.dot:

digraph "test root graph" {
    x -> xx [key=1];
x -> xxx [key=1];
xx -> x [key=2];
xx -> xxx [key=1];
xx -> xxxx [key=1];
xxx -> x [key=2];
xxx -> xx [key=2];
xxx -> xxxx [key=1];
}

As you see there is no trace of these stale edges in the dot representation. This seems like a bug to me, or am I doing something wrong?


The identifiers imagwrite and memDisc have to do with the custom IO discipline I'm using. If needed I can upload the discipline code as well.

ellson commented 7 years ago

I can reproduce this problem on Linux using this hacked C version

work:bug1199$ cat test.c
#include <string.h>
#include <stdlib.h>
#include <gvc.h>
#include <cgraph.h>
#include <cdt.h>

void dump(Agraph_t* root, char* filename)
{
    GVC_t* gvc = gvContext();
//    imagwrite(root, (filename + ".dot").c_str());
    gvLayout(gvc, root, "dot");
    gvRenderFilename(gvc, root, "png", filename);
}

void main()
{
    Agraph_t* root = agopen("test root graph", Agdirected, NULL);

    Agnode_t* n1 = agnode(root, "x", 1);
    Agnode_t* n2 = agnode(root, "xx", 1);
    Agnode_t* n3 = agnode(root, "xxx", 1);
    Agnode_t* n4 = agnode(root, "xxxx", 1);

    agedge(root, n2, n4, "1", 1);
    agedge(root, n3, n4, "1", 1);
    agedge(root, n1, n2, "1", 1);
    agedge(root, n2, n1, "2", 1);
    agedge(root, n1, n3, "1", 1);
    agedge(root, n3, n1, "2", 1);
    agedge(root, n2, n3, "1", 1);
    agedge(root, n3, n2, "2", 1);

    dump(root, "dump1.png");
    agdelete(root, n1);
    dump(root, "dump2.png");
}
work:bug1199$ cat Makefile
CFLAGS=-I/usr/include/graphviz
LDFLAGS=-lgvc -lcgraph -lcdt

test: test.c

Now the question is whats wrong with agdelete() ?

.

ellson commented 7 years ago

OK, I don't think its a problem with agdelete(). Adding: gvFreeLayout(gvc, root); gvFreeContext(gvc); seems to fix the problem.

work:bug1199$ cat test.c
#include <string.h>
#include <stdlib.h>
#include <gvc.h>
#include <cgraph.h>
#include <cdt.h>

void dump(Agraph_t* root, char* filename)
{
    GVC_t* gvc = gvContext();
//    imagwrite(root, (filename + ".dot").c_str());
    gvLayout(gvc, root, "dot");
    gvRenderFilename(gvc, root, "png", filename);
    gvFreeLayout(gvc, root);
    gvFreeContext(gvc);
}

void main()
{
    Agraph_t* root = agopen("test root graph", Agdirected, NULL);

    Agnode_t* n1 = agnode(root, "x", 1);
    Agnode_t* n2 = agnode(root, "xx", 1);
    Agnode_t* n3 = agnode(root, "xxx", 1);
    Agnode_t* n4 = agnode(root, "xxxx", 1);

    agedge(root, n2, n4, "1", 1);
    agedge(root, n3, n4, "1", 1);
    agedge(root, n1, n2, "1", 1);
    agedge(root, n2, n1, "2", 1);
    agedge(root, n1, n3, "1", 1);
    agedge(root, n3, n1, "2", 1);
    agedge(root, n2, n3, "1", 1);
    agedge(root, n3, n2, "2", 1);

    dump(root, "dump1.png");
    agdelete(root, n1);
    dump(root, "dump2.png");
}
emden commented 7 years ago

I believe it is necessary to call gvFreeLayout. This documented in http://www.graphviz.org/doc/libguide/libguide.pdf.

chtenb commented 7 years ago

I believe it is necessary to call gvFreeLayout. This documented in http://www.graphviz.org/doc/libguide/libguide.pdf.

You are entirely correct. Is it possible to check for an existing layout right before a new layout is computed? We could then free the existing one to prevent bugs like these from happening.