usnistgov / NetSimulyzer-ns3-module

A flexible 3D visualizer for displaying, debugging, presenting, and understanding ns-3 scenarios.
Other
33 stars 5 forks source link

Issue with "decoration" #27

Open igs3000 opened 2 years ago

igs3000 commented 2 years ago

The Netsimulyzer documentation shows the following way to use the "decoration" feature

auto decoration = CreateObject(/ the orchestrator /); decoration.SetAttribute ("Model", StringValue ("path/model.obj")); decoration.SetAttribute ("Height", DoubleValue (2.0));

But if we try to set a 3D object as decoration it ends with following error:,

error: ‘class ns3::Ptr’ has no member named ‘SetAttribute’ decoration.SetAttribute ("Model", StringValue("mymodel.obj"));

Then I managed to run it by using decoration-> reference operator as follows :

auto decoration = CreateObject(orchestrator); decoration->SetAttribute ("Model", StringValue("mount.blend1.obj")); decoration->SetPosition ({0.0,0.0,0.0}); //decoration->SetAttribute ("Height", DoubleValue (0)); decoration->SetAttribute ("Scale" , DoubleValue (100));

Now if I comment the “Height” attribute setting line, then it works and opened the model in the visualizer. But if I uncomment the line “Height” attribute setting line, then the compiler ends with following error: (In fact, as document says, I am trying to change height before changing the scale.

Build failed -> task in 'mytestsimulation failed with exit status 1 (run with -v to display more information)

  1. Why the 'decoration.SetAttribute. is not working? (dot)
  2. Why adjusting height of the decorative 3D model is not working?.

Charles Pandian

bpe2 commented 2 years ago

Hey Charles,

1) The "dot" syntax for setting an attribute is an error with the documentation. It should be the "arrow" syntax, as you pointed out. I've corrected this in the README and the Sphinx documentation.

2) The Height attribute used to be a DoubleValue, but now is an OptionalValue<double>, which was also incorrect in the documentation, and I've updated that as well.

Here's the correct way to set height on a Decoration:

decoration->SetAttribute ("Height", OptionalValue<double> (2.0));

I'm working on conversions for ns-3 types (like DoubleValue) to OptionalValues, but those are not quite done yet, so for the time being, you'll have to use the syntax above.

Thanks for finding that error. Let me know if this works for you.

Evan

igs3000 commented 2 years ago

Hello Evan,

I am receiving the following error while using it as per your suggestion. decoration->SetAttribute ("Height", OptionalValue (2.0));

../scratch/throughput-sink-example-netsimulyzer.cc: In function ‘int main(int, char**)’: ../scratch/throughput-sink-example-netsimulyzer.cc:155:39: error: ‘OptionalValue’ was not declared in this scope decoration->SetAttribute ("Height", OptionalValue (2.0)); ^~~~~ ../scratch/mytest.cc:155:39: note: suggested alternative: In file included from ./ns3/node-configuration.h:46, from ./ns3/orchestrator.h:48, from ./ns3/building-configuration.h:43, from ./ns3/building-configuration-container.h:38, from ./ns3/netsimulyzer-module.h:10, from ../scratch/mytest.cc:45: ./ns3/optional.h:59:7: note: ‘ns3::netsimulyzer::OptionalValue’ class OptionalValue : public AttributeValue ^~~~~ ../scratch/mytest.cc:155:53: error: expected primary-expression before ‘double’ decoration->SetAttribute ("Height", OptionalValue (2.0));

Another Issue Further, I think, scaling also not working as expected. In the following, I am trying to set the scale as 10 times the original size of the model. But, it is only showing a lower size model on NetSimulyzer.

decoration->SetAttribute ("Scale" , DoubleValue (10));

Can't we increase the size beyond the original size?

Charles Pandian.

bpe2 commented 2 years ago

For the first thing, unfortunately, OptionalValue is a templated type, so you must pass the contained type in angle brackets, like this:

OptionalValue<double> (2.0)
// This part ^^^^^^^^

As for the Scale attribute, the line you posted should increase the 3D model's size by a factor of 10. Here's an example using the provided server model using your scale factor (Scale of 1 is the default): scaled-decoration

I'll note that Decoration properties (aside from Position and Orientation) must be set before Simulation::Start() is called. Otherwise, the changes are ignored. Could that potentially be your issue? If not, could I ask for the JSON output file from your simulation to see if your new scale is being written?

Evan

igs3000 commented 2 years ago

Thank you for your prompt reply. I will try this and update you with the results.

Charles Pandian

igs3000 commented 2 years ago

Mr. Evan and bpe2,

For your information, 'Scale' is working good. (The reason for my misunderstanding was due to the abnormal height of the 3D model that I used)

But still, I am getting the same error while trying to set Height.

Since the simulation is just ending - it is not creating a complete .json file.

I tried everything. ============================The Error Output from the terminal =============

decoration->SetAttribute ("Height", OptionalValue (2.0));is giving error.

auto orchestrator = CreateObject auto orchestrator = CreateObject ("SimpleNS3FANETRandomWalk3D.json");

auto decoration = CreateObject(orchestrator); decoration->SetAttribute ("Model", StringValue("mount.blend1.obj")); decoration->SetPosition ({0.0,0.0,0.0}); decoration->SetAttribute ("Height", OptionalValue (2.0)); decoration->SetAttribute ("Scale" , DoubleValue (100));

ns-3.33# ./waf --run SimpleNS3FANETRandomWalk3D Waf: Entering directory `/home/ns-3.33/build' [1948/2009] Compiling scratch/SimpleNS3FANETRandomWalk3D.cc ../scratch/SimpleNS3FANETRandomWalk3D.cc: In function 'int main(int, char**)': ../scratch/SimpleNS3FANETRandomWalk3D.cc:72:39: error: 'OptionalValue' was not declared in this scope decoration->SetAttribute ("Height", OptionalValue (2.0)); ^~~~~ ../scratch/SimpleNS3FANETRandomWalk3D.cc:72:39: note: suggested alternative: In file included from ./ns3/node-configuration.h:46, from ./ns3/orchestrator.h:48, from ./ns3/building-configuration.h:43, from ./ns3/building-configuration-container.h:38, from ./ns3/netsimulyzer-module.h:10, from ../scratch/SimpleNS3FANETRandomWalk3D.cc:4: ./ns3/optional.h:59:7: note: 'ns3::netsimulyzer::OptionalValue' class OptionalValue : public AttributeValue ^~~~~ ../scratch/SimpleNS3FANETRandomWalk3D.cc:72:53: error: expected primary-expression before 'double' decoration->SetAttribute ("Height", OptionalValue (2.0)); ^~

Waf: Leaving directory `/home/ns-3.33/build' Build failed -> task in 'SimpleNS3FANETRandomWalk3D' failed with exit status 1 (run with -v to display more information)

===========================================================================

Used the same lines in different simulations - at the top - at bottom - same error.

Ok - I will try to update the NetSimulyzer ns-3 extension module and try it again and tell you about my success/failiure.

Charles Pandian,

igs3000 commented 2 years ago

Hello Evan and bpe2, It is working.

Sorry. It was my mistake. I didn't used Netsimulyzer name space. Now setting "height" is working.

The clue was in the error message itself : ../scratch/SimpleNS3FANETRandomWalk3D.cc:72:39: error: 'OptionalValue' was not declared in this scope

But for your information, all other features of Netsimulyzer are working even without using Netsimulyzer name space.

Anyway, thank you for your support.

Charles Pandian

igs3000 commented 2 years ago

Hello Evan and bpe2,

Setting Height is working without any error. But, the output is not as I expected.

I am trying to set a landscape image as a 'simulation playground' image (to cover the entire ground area).

For that I am loading a 3D mountain landscape model and Setting its size as 100 times the original. Now the mountains also becomes very high, but covering the area as I expected.

Now I am trying to reduce the height of the model without reducing its x-y coverage area.

Instead of getting a flat or height reduced 3D model, again I am getting a very small, scaled down 3D model on the playground. Now it is not covering the entire area.

And for your information, the order of the following lines didn't make anything difference in behaviour.(whether setting height after scale or scale after height)

decoration->SetAttribute ("Height", OptionalValue (0.1)); decoration->SetAttribute ("Scale" , DoubleValue (100));

Am I understanding the "Height" attribute wrongly? or missing something here?

Charles Pandian

bpe2 commented 2 years ago

Hey Charles,

The Height attribute tells the application to calculate a scale factor for the given model, so it ends up being the specified height. This performs a "uniform scale," or a scale on each axis (not just up/down) so everything doesn't look stretched. Think of it as just using the Scale attribute without having to calculate an appropriate value manually.

As an example, the values you provided here will: 1) Tell the applicated to scale the whole model, so it ends up being 0.1 units tall (very small) 2) Increase the size of that result by 100x (since Scale is applied after Height)

Unfortunately, there isn't a mechanism to perform scaling on just one axis. However, if this is a common use case for you, I could add one in without too much difficulty. Let me know if you're interested.

I'll admit, the documentation should be expanded to clarify what exactly is happening here. I'll do that as well.

Evan

igs3000 commented 2 years ago

Hello Evan,

So, I just misunderstood the scope of implementation of the “height” attribute. Ok. Thank you for the clarification.

Charles Pandian,

On Tue, Oct 19, 2021 at 11:29 PM Evan Black @.***> wrote:

Hey Charles,

The Height attribute tells the application to calculate a scale factor for the given model, so it ends up being the specified height. This performs a "uniform scale," or a scale on each axis (not just up/down) so everything doesn't look stretched. Think of it as just using the Scale attribute without having to calculate an appropriate value manually.

As an example, the values you provided here will:

  1. Tell the applicated to scale the whole model, so it ends up being 0.1 units tall (very small)
  2. Increase the size of that result by 100x (since Scale is applied after Height)

Unfortunately, there isn't a mechanism to perform scaling on just one axis. However, if this is a common use case for you, I could add one in without too much difficulty. Let me know if you're interested.

I'll admit, the documentation should be expanded to clarify what exactly is happening here. I'll do that as well.

Evan

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/usnistgov/NetSimulyzer-ns3-module/issues/27#issuecomment-946966838, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEXK5MEGYVEULSOQY4PCA23UHWWXHANCNFSM5GG64XEQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

igs3000 commented 2 years ago

Hello Evan and bpe2,

Just I want to share the information about a change that I did in Model.cpp of "NetSimylyzer 3D Visualization tool" to change the behavior of the attribute "Height" of "decorations"

In the function void rebuildModelMatrix() , I just added an if-else condition for implementing the behavior that I wanted.

if(targetHeightScale==1)
      modelMatrix = glm::scale(modelMatrix, {scale, scale, scale});
  else
          modelMatrix = glm::scale(modelMatrix, {scale, targetHeightScale, scale});

Now, we can change the “height” of the loaded decoration model irrespective of its “scale”.

But for your information, you are doing some kind of scaling on targetHeightScale - so that, I have to set the “Height” attribute value to little high value. Even we can disable that scaling on targetHeightScale which is happening at Decoration.cpp.

this->model.setTargetHeightScale(*ns3Model.height / height);

Of course, I am not understanding the reason for this scaling that you did on 'Height'.

But , even with this scaling on "Height", I can change the heigh of my 3D model as per my wish - so I can live with it. :-) :-))

Just sharing this - thats all.

Thank you.

Charles Pandian

bpe2 commented 2 years ago

Hey Charles,

I'm glad you were able to find what you needed. I've opened a new issue to add non-uniform scaling, as well as also allowing width or depth to be used to calculate a target uniform scale. It's issue #28. Let me know if this would satisfy your needs here as well.

Evan

igs3000 commented 2 years ago

Hello Evan,

Yes. This kind of dimension specific scaling will be useful to modify the original shape/dimension of 3D model as per our need.

Suggestion: But, we may avoid this much of the confusion simply by passing three parameters to the "Scale" attribute.

So that, the following examples of lines will scale model in all three dimensions.

decoration->SetAttribute ("Scale" , DoubleValue (1), DoubleValue (1), DoubleValue (1));
decoration->SetAttribute ("Scale" , DoubleValue (0.5), DoubleValue (0.5), DoubleValue (0.5));
decoration->SetAttribute ("Scale" , DoubleValue (10), DoubleValue (10), DoubleValue (10));

So that, the following examples of lines will scale model independently in the three dimensions.

decoration->SetAttribute ("Scale" , DoubleValue (1), DoubleValue (0.5), DoubleValue (0.7));
decoration->SetAttribute ("Scale" , DoubleValue (100), DoubleValue (25), DoubleValue (50));

====================================================== So, simply one attribute setting is enough. decoration->SetAttribute ("Scale" , DoubleValue (1), DoubleValue (1), DoubleValue (1));

And of course, we may remove all the unnecessary things to avoid confusion and the complexity in the code:

decoration->SetAttribute ("Height", OptionalValue (1));

and any future attribute settings like :

decoration->SetAttribute ("Width", OptionalValue (1));
decoration->SetAttribute ("Depth", OptionalValue (1));
decoration->SetAttribute ("KeepAspectRatio", bool);

should be avoided.

Just a suggestion. You may ignore it if you are having other future planes in this regard.

Charles Pandian

bpe2 commented 2 years ago

Hey Charles,

I did also consider expanding the Scale attribute similarly to the way you described here. It would have functioned similarly to the Orientation property. However, I don't think it would function as a totally 'drop in' replacement, as Vector3DValue cannot be constructed from a DoubleValue (or a list of DoubleValues) like in your example. I also don't think ns-3 attributes may be overloaded, so I couldn't just add an additional Vector3DValue with the same name.

In short, this would require an API break unless you know of an ns-3 attribute type that may be set like you're describing here (and if you do, please let me know). I'm alright with API breaks, and I think your suggestion makes sense for one. This will likely come up in the next version I'm willing to introduce API breaks.

Evan

igs3000 commented 2 years ago

In fact I do not understand what you mean by "API break" here. Because, simply, we are going to pass a 3d vector instead of a double.

Whether we are going to pass a single 3d vector or separate three double values, we are going to pass it to a glm::scale function (in the 3D tool part)

So, what ever we change in the ns-3 extension module, additionally we need to modify some parts of the NetSimulyzer 3D visualization tool also.

So (instead of handling four separate thing height, width, depth, aspect ratio) doing one change on Scale Attribute will minimize a lot of coding in ns-3 extension module as well as in the NetSimulyzer 3D visualization tool. Most importantly, it will avoid confusion to the End-user.

bpe2 commented 2 years ago

The API break part I'm referring to would be in the module, specifically anything using the attribute (or the NodeConfigurationHelper) and not the setter function directly.

using netsimulyzer;

auto decoration = CreateObject<Decoration>(/*orchestrator*/);

// This (currently working code) would now break at runtime
// If I changed `Scale` to a `Vector3DValue`
decoration->SetAttribute("Scale", DoubleValue(10.0));

NodeConfigurationHelper nodeHelper{/*orchestrator*/};

// This too if I allow non-uniform scaling on Nodes as well
nodeHelper.Set("Scale", DoubleValue(10.0));

The SetScale() functions on their own I could absolutely overload and not introduce an API break. Perhaps a middle ground could be found where there are convenience functions that allow for setting all three of the scaling attributes while leaving the separate attribute, at least for now. Maybe something like what you proposed earlier:

using netsimulyzer;

auto decoration = CreateObject<Decoration>(/*orchestrator*/);

// Old uniform scale
decoration->SetScale(10.0);

// Non-uniforn scale
decoration->SetScale(1.0, 10.0, 1.0);

I can't touch the attributes just yet, but does this make sense? I could do the same with the 'target' type scales proposed #28 too.

I'm not worried about the application side of this, since the user doesn't interact with the API there, and I could maintain compatibility with the previous single value scale in the JSON output, so the ns-3 module is the only concern here.

Evan

igs3000 commented 2 years ago

Hello Evan,

// Old uniform scale
decoration->SetScale(10.0);

// Non-uniforn scale
decoration->SetScale(1.0, 10.0, 1.0);

Yes. The above will be a good temporary solution.

But in the .json file, you are sending parameters to the visualization tool for setting the initial "decorations" "decorations":[{"height":100,"id":1,"model":"mount.blend1.obj","orientation":{"x":0.0,"y":0.0,"z":0.0},"position":{"x":300.0,"y":300.0,"z":0.0},"scale":300.0,"type":"decoration"}], And of course the same for individual nodes also.

In that, you are passing height(double) and scale(double) only. So if you add independent scaling, you will add another value for another dimension.
So, what ever we change here in ns-3 extension should have the same impact on the 3D visualization tool part also.

So, whatever the changes we do here (ns3-extension) now should be a permanent fix for this problem. So that, we should not need any major change in the 3D visualization tool part code in future.

Just sharing my thoughts thats all.

Charles Pandian

bpe2 commented 2 years ago

Hey Charles,

As far as output is concerned, this is how I thought to do it.

For the 'scale' property

[{
  // Old style, produces a uniform scale
  "scale": 300.0,
  // ...
},
{
  // New style, non-uniform scale (unless all the values match of course)
  "scale": [1.0, 300.0, 1.0]
}]

It's pretty easy to support both styles when loading this in the application.

The 'height' property would probably be replaced by something slightly more complex, maybe something like this:

[{
  // optional
  "target-scale": {
    // Each of these should also be optional
    // Depending on if they're set in the module
    "x": 10.0,
    "y": 5.0,
    "z": 6.0
  },
  // ...
}]

I'll still have to support the old 'height' property for a while since I prefer output produced with an older version of the module continue to work, although that wouldn't be too much trouble.

Does this make sense with your comment? I do appreciate that you're thinking this the whole way through.

Evan