Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Map colors looks wrong/inverted on npm package, but ok for compiled version #574

Open
wakoko79 opened this issue Oct 17, 2022 · 6 comments
Labels

Comments

@wakoko79
Copy link

Description
Map colors looks wrong/inverted on npm package, but ok for compiled version.

  • Library Version: 0.15 (?, from compiled min), 0.17 (?, package from npm) 1.1.0
  • ROS Version: Noetic
  • Platform / OS: Ubuntu 20.04 (google chrome browser)

Longer Description
So I tried to use ros3d, we first tried the tutorial from ROS.
There are multiple imports I tried, and the only version that has correct colors is from the compiled full/min import from robotwebtools.
We are using reactjs for this.

Here is a snippet for the react component (ros3d was imported in index.html which is not shown here):

// import ROSLIB from 'roslib';
// import {Viewer,OccupancyGridClient} from 'ros3d';
// import {Ros} from 'roslib';
class Index extends Component{
    state={

    };
    constructor(){
        super();
        this.init_connection=this.init_connection.bind(this)
    }

    componentDidMount(){
        this.init_connection()
    }

    init_connection() {
        var ros = new window.ROSLIB.Ros({
            url : 'ws://localhost:9090'
        });

         // Create the main viewer.
        var viewer = new window.ROS3D.Viewer({
        divID : 'map',
        width : 800,
        height : 600,
        antialias : true
      });
  
      // Setup the map client.
      var gridClient = new window.ROS3D.OccupancyGridClient({
        ros : ros,
        rootObject : viewer.scene,
        // color : {r:255,g:0,b:0}
      });
    //}
    }
    render(){
        //const map = this.init_connection();
        return(
            
            <div onLoad={this.init_connection}>
                <h1>Simple Map Example</h1>
                <div id="map"></div>
            </div>
        )
    }
}
export default Index;

When importing from node_modules/ros3d, I just uncomment the import line above then change the names for Viewer, OccupancyGridClient, etc.

If I import from package from npm (that has pre-built files) by install with npm i ros3d, the colors seems to be inverted.
This is also true when I tried to install directly from github with npm i RobotWebTools/ros3djs.
When I also tried to import from the recommended links in the read me from jsdelivr and from github release, it is also inverted.

I would like to use the latest one (from github or npm) but it has wrong colors.

So, is there a change of usage of the package that is not published in ROS?
Or have I done anything wrong?
Why is the release from robotwebtools different from the one in github and npm?

Here are some screenshots.
From importing robotwebtools link1_full or link2_min:
from _ importing robotwebtools cdn

From importing from npm package (npm i ros3d, or from this github repository, npm_page
from _ npm i ros3d

@wakoko79 wakoko79 added the bug label Oct 17, 2022
@MatthijsBurgh
Copy link
Contributor

A lot of questions...

You are now comparing different versions of the library from different sources. Which isn't a good way to find the bug. So I suggest you stick to one source and then check different releases. So we can determine when the bug was introduced.
Don't go for the CDN as this doesn't contain all versions. So go either for npm or github.

Good luck. I am curious to the results.

@wakoko79
Copy link
Author

I found the issue.

Old code (OccupancyGrid class) transforms occupancy value [0, 100] to a grayscale value of [0, 255]; current code directly uses occupancy value to get RGBA values directly, which is wrong.

Here is a snippet of the old code for generating RGB values from occupancy data:

for ( var row = 0; row < height; row++) {
  for ( var col = 0; col < width; col++) {
    // determine the index into the map data
    var mapI = col + ((height - row - 1) * width);
    // determine the value
    var data = message.data[mapI];
    var val;
    if (data === 100) {
      val = 0;
    } else if (data === 0) {
      val = 255;
    } else {
      val = 127;
    }

    // determine the index into the image data array
    var i = (col + (row * width)) * 3;
    // r
    imageData[i] = (val * color.r) / 255;
    // g
    imageData[++i] = (val * color.g) / 255;
    // b
    imageData[++i] = (val * color.b) / 255;
  }
}

The specific code for transforming occupancy values to grayscale values:

var data = message.data[mapI];
var val;
if (data === 100) {
  val = 0;
} else if (data === 0) {
  val = 255;
} else {
  val = 127;
}

In the new code, we have:

var val = this.getValue(mapI, invRow, col, data);
// determine the color
var color = this.getColor(mapI, invRow, col, val);

And as you can see in the definition, it straight up uses the occupancy value as:

ROS3D.OccupancyGrid.prototype.getColor = function(index, row, col, value) {
return [
(value * this.color.r) / 255,
(value * this.color.g) / 255,
(value * this.color.b) / 255,
255
];
};

The fix for this should be easy. Just put the code for transforming occupancy value to grayscale value in getColor(), then use that to get RGBA values.
But I'm not sure if this was the intended behavior or not.

@MatthijsBurgh
Copy link
Contributor

MatthijsBurgh commented Oct 19, 2022

Related PRs: #235, #339 and #343.

I think we both agree the alpha channel is not the problem here. But the problem is that the colour is inverted anymore.

The old implementation only allowed 3 levels (white, gray and black). I think we should come up with a function which makes this continues. Something like MAX_OUTPUT*(MAX_INPUT-x). I will try to work on this in the next week or two.

@wakoko79 could you test if the current opacity option of the OccupancyGrid does work? Otherwise we could fix that too.

@wakoko79
Copy link
Author

wakoko79 commented Oct 19, 2022

Related PRs: #235, #339 and #343.

I think we both agree the alpha channel is the problem here. But the problem is that the colour is inverted anymore.

The old implementation only allowed 3 levels (white, gray and black). I think we should come up with a function which makes this continues. Something like MAX_OUTPUT*(MAX_INPUT-x). I will try to work on this in the next week or two.

@wakoko79 could you test if the current opacity option of the OccupancyGrid does work? Otherwise we could fix that too.

No, the problem is not the alpha channel. (edit: still true, but quote has typo)

The alpha channel was properly handled.

The issue lies with the value in:

ROS3D.OccupancyGrid.prototype.getColor = function(index, row, col, value) {
return [
(value * this.color.r) / 255,
(value * this.color.g) / 255,
(value * this.color.b) / 255,
255
];
};

The equations there for conversion of value implies that value should be a "brightness" of a RGBA component, say R.
In other words, value is a grayscale value 0~255, with 255 as the brightest as it is white.
That is the implication since this.color.r is an integer, with default value of 255.

Now, occupancy data from ros message is 0 for not occupied, and 100 for surely occupied.

And from intuition, an occupied pixel should be black.
So you have to convert the occupancy data first to brightness (like old code does), then use that converted value for calculation of RGB values.

The reason that the current code inverts the colors is that getColor() directly uses occupancy data, instead of transforming it first to a grayscale value.

For it to work as before, getColor() should go something like this:

ROS3D.OccupancyGrid.prototype.getColor = function(index, row, col, value) {
  var val_trans = value;
	    if (value === 100) {
	      val_trans = 0;
	    } else if (value === 0) {
	      val_trans = 255;
	    } else {
	      val_trans = 127;
	    }
  return [
    (val_trans * this.color.r) / 255,
    (val_trans * this.color.g) / 255,
    (val_trans * this.color.b) / 255,
    255
  ];
};

On the other note @MatthijsBurgh, opacity is fine.

@MatthijsBurgh
Copy link
Contributor

MatthijsBurgh commented Oct 19, 2022

@wakoko79 I made a typo in my previous message.

The alpha channel is indeed not the problem. Could you edit your last message based on this.

P.S. thanks for testing the opacity.

@jmnavarrete
Copy link

jmnavarrete commented Dec 18, 2023

In my case using npm and downloading the component from github, the occupancyGrid color is still inverted. Maybe the ros3d.cjs.js has not been updated?
I have been able to solve the problem by introducing the following code in my react application:
`

    Ros3Djs.OccupancyGrid.prototype.getColor = function (index, row, col, value) {
      return [
        255 - ((value * this.color.r) / 255),
        255 - ((value * this.color.g) / 255),
        255 - ((value * this.color.b) / 255),
        255
      ];
    };

`

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants