NCAR / VAPOR

VAPOR is the Visualization and Analysis Platform for Ocean, Atmosphere, and Solar Researchers
https://www.vapor.ucar.edu/
BSD 3-Clause "New" or "Revised" License
178 stars 49 forks source link

Params class to indicate if an entry is updated since last read. #2583

Open shaomeng opened 3 years ago

shaomeng commented 3 years ago

Currently, in most of the renderers, the following workflow is adopted to decide when to update a rendering: 1) Keep a cached version of param entries. 2) Upon a paint event, compare the cached params to the ones retrieved from param class. 3) Based on compare results from step 2), decide if a full re-paint, partial repaint, or any other actions are appropriate.

This workflow accommodates the current design of params class that they only remember the value of an entry. This workflow also has a drawback which is that every renderer needs to implement a local cache and comparison mechanism, leading to code repetition and chance for error.

A proposed enhancement is to have the params class indicate if an entry is updated since last read. By having the params class doing it, a renderer can simply query the status of param entries and decide its actions.

Note: in VAPOR a common use case is that two clients can query the same params instance, namely the renderer and the GUI. In this use case, this updated since last read flag should reflect if it's read by the renderer, but not the GUI.

The following pseudo code should demonstrate the proposed enhancement with special considerations to a renderer vs. GUI query distinction.

void Params::SetValueLong( std::string tag, long val ) {
    _local_tag = tag;
    _local_val = val;
    _update_flag = true;  // This is the proposed change: this entry has a brand new value now.
}

// When a GUI widget queries, pass "true" for the 2nd argument.
// When a renderer queries, pass "false" for the 2nd argument
std::pair<bool, long> Params::GetValueLong( std::string tag, bool gui_query ) {
    if( gui_query ) {
        return { _update_flag, _local_val };
    }
    else {
        bool old_flag = _update_flag;
        _update_flag = false;  // Since this value is accessed now, so mark it false.
        return { old_flag, _local_val };
    }
}

void Renderer::paingGL() {
    auto flowLength = params->GetValueLong( tag );
    if( flowLength.first ) {
        // This condition means that the flow length entry is updated since last paint, 
        // so we need do work to reflect the change.
    }
}
clyne commented 3 years ago

@shaomeng this is a good idea and we actually had this in VAPOR 2. It was, unfortunately, not migrated when we refactored the params data base for VAPOR3. The issue is more complex than it appears, and the solution you propose above probably would not prove satisfactory. The difficulty is that there are multiple "clients" of the params data base that need to retrieve param values. Hence, resetting a single update flag whenever a value is fetched with GetValue* won't cut it. You need to have multiple update flags, one for each client. The way we addressed this in VAPOR2 was to allow clients of the params database to register flags owned by the client that would get set to true every time SetValue was called for a particular parameter. It was then the responsibility of the client to clear the flag. In pseudo code you might have something like:

Params::RegisterUpdateFlag(string tag, bool &myflag) {
  AllFlags[tag].push_back(myflag);
}
Params::SetValueLong(string tag, long val) {
.
.
  for (auto f: AllFlags[tag])
    f = true
  }
}

Render::Inititialize() {
  RegisterUpdateFlag("isovalue", _isoChanged)
}
Render::paintGL() {
  if (_isoChanged) {
    newval = params->GetValue("isovalue")
    _isoChanged = false;
 }
}

bool isoValueFlag;

shaomeng commented 3 years ago

It looks like the challenge is posed by a multi-client situation. But throughout VAPOR, how often that scenario occurs? My understanding is that every renderer has its own params instance, and even if two flow renderers are created, they still have separated params instances, is it right?

clyne commented 3 years ago

In general there are at least two "clients" for every render parameter: the renderer, and the GUI which needs to display the parameter value to the user

shaomeng commented 3 years ago

Oh this particular two clients scenario is exactly what I was thinking, and will benefit from an update flag, because there's still only one client who cares if the param value is updated or not. This client is the renderer.

The renderer cares about it because param update can cause expensive operations, so a renderer needs to rely on this update flag to do things in a smart way to potentially save costs.

The GUI doesn't care about param update flags though, because updating a GUI widget is so cheap, that you can always take a brand new value from Params and display that value.

Note: We need to make sure that the GUI queries the params without resetting the flag, while the renderer do. I'll update the original pseudocode to reflect it.