[ghemical-devel] Suggested View Optimization

[Tommi Hassinen]

On Fri, 3 Mar 2006, Donald Ephraim Curtis wrote:

> I would like to suggest that AtomUpdateItem should take as a parameter a 
> GtkTreeIter, then AtomUpdate can take as input an atom, find it's 
> corresponding iterator, then call AtomUpdateItem.
>
> In this way we can alleviate the O(n^2) we're getting when calling 
> AtomUpdateAllItems.

Yes, the atom list can grow large, and this can become a problem.

Before I have not had need for AtomUpdateAllItems(), since I was able 
handle everything using the following operations:

 	1) a new atom is added -> add it to the list.

 	2) an atom is modified (element changed etc) -> find it from
 	the list and make the list show the new value.

 	3) an atom is deleted -> find it from the list and remove it.

Therefore I never had the need to update the full list. Perhaps the 
"frozen atoms" feature could be made to work the same way?

If I look for AtomUpdateAllItems() from the source, this is the only place 
where it is called:

void gtk_project_view::Update(bool directly)
{
 	AtomUpdateAllItems();
}

So it's the "Update" method of a "view" class. Unfortunately this kind of 
arrangement is obsolete in the cvs HEAD version. There the "view" classes 
can only use OpenGL to display their content, not for example gtk widgets 
that is the case above. So, in cvs HEAD version these atom/bond lists can 
(and must) be done separately from the "view" classes.

In my opinion it's best to avoid using AtomUpdateAllItems() in the first 
place ; so we could comment the above code out, and make sure the line 
which user has clicked will be updated instead of the full list. I think 
the following should do the trick ; this is from 
gtk_project_view::atoms_ToggleLocked()

 	atom * p1 = (* it)->ref;
 	p1->SetLocked(!(p1->GetLocked()));
 	gtk_list_store_set(GTK_LIST_STORE(data), &iter, 2, p1->GetLocked(), -1);
 	// gpv->AtomUpdateItem(p1);

 	printf("Locked Changed: %s\n", p1->GetLocked() ? "true":"false");

I think the solution is already above, it's just commented out ; 
immediately call AtomUpdateItem() once for the correct line, and that's 
all. Seems to work for me here... I could try this more tomorrow, and port 
it cvs HEAD too (should work there as well).

> Also, i think it's a good idea to add a hidden G_POINTER_TYPE column so that 
> rather than having to iterate through the gpv_atom_records when we have a 
> selected row, we can simply use that hidden field to give us a pointer to our 
> record.  (linking both ways).

A hidden field sounds good, although I have never used them so far. Sure 
this could be an alternative solution.

Regards,

 	Tommi