ibgreen Github contribution chart
ibgreen Github Stats
ibgreen Most Used Languages

Activity

26 Sep 2022

Ibgreen

Revert "Add banner for Madrid Summit 2022 (#7272)"

This reverts commit a40ed18833b13caf2b92c03b9b584664257e5e94.

Merged On 26 Sep 2022 at 04:46:12

Ibgreen

Do we have links to the information (recordings, examples etc) somewhere on the site?

Commented On 26 Sep 2022 at 04:46:12

Ibgreen

chore(i3s): export more types

Also fixed one type. Spec - https://github.com/Esri/i3s-spec/blob/master/docs/1.8/histogram.cmn.md

Merged On 26 Sep 2022 at 02:55:12

Ibgreen

Just consider that every exported type becomes part of the API and changing it becomes a breaking change. It also needs to good docs (at least JSDocs) and ideally we should have minial test cases making sure it the type is defined by the library.

Commented On 26 Sep 2022 at 02:55:12

Ibgreen

fix(gltf): add diffuseTex support to v1.0
Merged On 19 Sep 2022 at 03:54:22

Ibgreen

Commented On 19 Sep 2022 at 03:54:22

Ibgreen

chore(webgl): separate mem stats another way

Fix an issue with adding new fields to gl context object in firefox and mobile safari.

Merged On 19 Sep 2022 at 03:52:32

Ibgreen

Commented On 19 Sep 2022 at 03:52:32

Ibgreen

feat(i3s): colorize by attribute

colorsByAttribute option for attribute-driven visualization: image

Merged On 19 Sep 2022 at 03:44:27

Ibgreen

Commented On 19 Sep 2022 at 03:44:27
Issue Comment

Ibgreen

Rename to carto-pydeck and use carto-auth

Change List

  • Rename pydeck-carto project to carto-pydeck
  • Use carto-auth as a dependency (remove the CartoAuth code)
  • Review tests and examples

Note: merge this PR https://github.com/visgl/deck.gl/pull/7255 first.

Forked On 16 Sep 2022 at 04:45:10

Ibgreen

If we wanted to formalize python package naming (good idea regardless of whether we proceed with this "exception") we could look into the name scoping semantics in the Python package managers. I suspect that @kylebarron has sufficiently deep understanding of how things work here (e.g. pydeck.modulename)

Commented On 16 Sep 2022 at 04:45:10
Issue Comment

Ibgreen

Rename to carto-pydeck and use carto-auth

Change List

  • Rename pydeck-carto project to carto-pydeck
  • Use carto-auth as a dependency (remove the CartoAuth code)
  • Review tests and examples

Note: merge this PR https://github.com/visgl/deck.gl/pull/7255 first.

Forked On 16 Sep 2022 at 04:42:12

Ibgreen

The CARTO module is obviously unique right now, it is a company specific module that is actively being developed as open source directly in the vis.gl repos, and that is of course very cool.

So it is not surprising that it is pushing some of our existing conventions, in this case by being the first module to abandon our long standing module naming conventions.

By that I mean that for all vis.gl javascript modules, we have been using the @deck.gl/ organization prefix on all module names to signal that these are legit packages, guaranteed to be published by the vis.gl team. No one else can publish a module starting with @deck.gl/... and the name prefix shows that the package is officially supported by vis.gl community.

My assumption has been that we would follow similar conventions in Python (and to date we have been using the pydeck- prefix).

Obviously, this PR breaks this pattern,. While I can understand the wish to do so from a marketing perspective, by making this change, we are losing some things:

  • we dilute the implicit promise and easy recognizability of existing vis.gl module naming schemes.

  • we open for other integration modules to request make the same change, e.g. put their company names first. Say google-earthengine-pydeck and by extension @mapbox/deck.gl etc. It would now be harder to say no to such requests, once we have this precedent.

Somehow the new name is signaling that this is a primarily a CARTO owned and published module rather than a vis.gl owned and published integration module. As much as I appreciate the carto module being developed in the open in the deck.gl repo, one could argue that the source code for something named carto-pydeck should perhaps not be located in the deck.gl repo.

I am not hard opposed to this PR, but I did want to state my concerns (and TSC should be a great forum if we did want to discuss how to balance such things).

Commented On 16 Sep 2022 at 04:42:12

Ibgreen

fix(tile-converter): comment out transform attributes worker
Merged On 14 Sep 2022 at 07:19:51

Ibgreen

Commented On 14 Sep 2022 at 07:19:51
Merge

Ibgreen

fix(tile-converter): comment out transform attributes worker
Forked On 14 Sep 2022 at 07:19:47

Ibgreen

Usually the case when the result is not all in typed array form
On 14 Sep 2022 at 07:19:47

Ibgreen

fix(tile-converter): comment out transform attributes worker
Merged On 14 Sep 2022 at 07:19:48

Ibgreen

Commented On 14 Sep 2022 at 07:19:48
Issue Comment

Ibgreen

Add queryParams option to Tiles3DLoader

Some tile servers require query parameters to be appended to requests when requesting 3D tiles, a common one being adding an API key. Currently, this is only achievable in deck.gl by using onTilesetLoad in a rather hacky way:

new Tile3DLayer({
  id: 'tile-3d-layer',
  data: TILESET_URL,
  onTilesetLoad: tileset3d => {
     tileset3d._queryParams = {key: API_KEY};
  }
}); 

This PR adds an option to Tiles3DLoader to support passing the required query parameters directly:

new Tile3DLayer({
  id: 'tile-3d-layer',
  data: TILESET_URL,
  loadOptions: {
    '3d-tiles': {
      queryParams: {key: API_KEY}
    }
  }
}); 

Forked On 14 Sep 2022 at 01:34:50

Ibgreen

We have more multi file loaders and surely the same problem will arise in other placesThough doing it in a general way (top level load option) will of course require more thought.

Commented On 14 Sep 2022 at 01:34:50
Issue Comment

Ibgreen

Add queryParams option to Tiles3DLoader

Some tile servers require query parameters to be appended to requests when requesting 3D tiles, a common one being adding an API key. Currently, this is only achievable in deck.gl by using onTilesetLoad in a rather hacky way:

new Tile3DLayer({
  id: 'tile-3d-layer',
  data: TILESET_URL,
  onTilesetLoad: tileset3d => {
     tileset3d._queryParams = {key: API_KEY};
  }
}); 

This PR adds an option to Tiles3DLoader to support passing the required query parameters directly:

new Tile3DLayer({
  id: 'tile-3d-layer',
  data: TILESET_URL,
  loadOptions: {
    '3d-tiles': {
      queryParams: {key: API_KEY}
    }
  }
}); 

Forked On 14 Sep 2022 at 01:34:38

Ibgreen

I know the 3D tiles team has been proposing a similar PR in the past.

I have been slow to approve mainly because I would prefer to have a generic solution for query params in loaders.gl core, rather than keep slapping things onto the already exceedingly complex 3d tiles code base.

Also there has been some work on it here https://github.com/visgl/loaders.gl/pull/807/files I suppose that is more along the lines of preserving params in the URL rather than allowing them to be injected with an additional loader option.

Commented On 14 Sep 2022 at 01:34:38

Ibgreen

fix: tests (#2251)

Pushed On 13 Sep 2022 at 05:11:32

Ibgreen

fix: tests

Created On 13 Sep 2022 at 05:11:32

Ibgreen

Add banner for Madrid Summit 2022

Banner for Madrid Summit 2022

Desktop

image

Mobile

image

Merged On 13 Sep 2022 at 05:09:49

Ibgreen

Commented On 13 Sep 2022 at 05:09:49
Merge

Ibgreen

feat(tile-converter): parallel siblings conversion
Forked On 12 Sep 2022 at 03:24:08

Ibgreen

We have a `NumericArray` type in math.gl that also covers typed arrays.
On 12 Sep 2022 at 03:24:08

Ibgreen

feat(tile-converter): parallel siblings conversion
Merged On 12 Sep 2022 at 03:26:57

Ibgreen

Commented On 12 Sep 2022 at 03:26:57

Ibgreen

feat(tile-converter): parallel siblings conversion
Merged On 12 Sep 2022 at 03:26:57

Ibgreen

Commented On 12 Sep 2022 at 03:26:57

Ibgreen

fix(website): revert breaking commit

I reverted https://github.com/visgl/loaders.gl/pull/2192 . I breaks the website

Merged On 12 Sep 2022 at 03:22:34

Ibgreen

Commented On 12 Sep 2022 at 03:22:34

Ibgreen

chore(tile-conveter): i3s - geometry conversion tests
Merged On 12 Sep 2022 at 03:22:00

Ibgreen

Commented On 12 Sep 2022 at 03:22:00

Ibgreen

chore(tile-conveter): i3s - geometry conversion tests
Merged On 12 Sep 2022 at 03:22:00

Ibgreen

Commented On 12 Sep 2022 at 03:22:00
Merge

Ibgreen

chore(tile-conveter): i3s - geometry conversion tests
Forked On 12 Sep 2022 at 03:21:18

Ibgreen

Declare and document the constants above the function JSDoc
On 12 Sep 2022 at 03:21:18
Issue Comment

Ibgreen

SAO

Target Use Case

Hello! I Think ambient occlusion in deck.gl can significantly improve appearance of 3d geometry, such as buildings However, I didn't find any working solution ased on deck.gl or luma.gl

Proposal

Is there any opportunity to implement SAO in deck.gl? Screenshot_52

Forked On 09 Sep 2022 at 06:48:58

Ibgreen

Hmm, there was an SSAOPass (ssao.js) in v7 of luma.gl https://github.com/visgl/luma.gl/tree/7.1-release/modules/effects/src/experimental/passes, but I can't find it in the latest luma.gl version.

In v8, maintainers cut out a number of things they didn't feel were supported well enough. This may have been a victim of that cleanup.

I'd love to see it back, but I am Not sure if that module still works as is, or if it also needs the supporting Pass classes.

Commented On 09 Sep 2022 at 06:48:58
Issue Comment

Ibgreen

ReferenceError: Response is not defined @loaders.gl/core/dist/es5/lib/api/select-loader.js:

HI I am trying to Convert from /to 3Dtiles and i3s SLPK .

module.exports = { convertFiles: async (url, outputPath) => { const converter = new I3SConverter(); const tilesetJson = await converter.convert({ inputUrl: url, outputPath: "data", slpk: true, tilesetName: "BatchedColors", }); }, };

But I am getting error

UnhandledPromiseRejectionWarning: ReferenceError: Response is not defined node_modules/@loaders.gl/core/dist/es5/lib/api/select-loader.js:185:23)

Forked On 08 Sep 2022 at 09:11:34

Ibgreen

Are you on Node.js? You'll want to install @loaders.gl/polyfills

Commented On 08 Sep 2022 at 09:11:34

Ibgreen

Upgrade React example to the latest version

The current version of unfolded is 1.1.0 while the example is based on 0.3.1

Please upgrade. For example the latest version doesn't recognize setLayerVisibility

Forked On 08 Sep 2022 at 03:15:37

Ibgreen

Thanks for reporting! We just released 1.0.0 still working on examples. Shouldn't be long now.

Commented On 08 Sep 2022 at 03:15:37
Issue Comment

Ibgreen

Bench: Async benchmark tests run very slow

Background: I'm running a benchmark test where I created an async function with a 5 second delay. The benchmark took 178s on average to run. If I lower the delay to 1 second, the benchmark took 30 seconds, when I increase the delay to 10 seconds, the benchmark took 318s.

Question: I'm curious why a 5 second function took 3 minutes to run, or a 10 second function took 5+ minutes to run. Is this a bug or is Bench intended to take this long?

Note: I noticed after further research that runBenchTestForMinimumTimeAsync has a multiplier of 10 by default on line 306 in bench.ts and it cannot be changed. Is there anything that can be done to be able to control that?

Forked On 06 Sep 2022 at 08:36:22

Ibgreen

I think it is _throughput, it was used in loaders.gl

Commented On 06 Sep 2022 at 08:36:22
Issue Comment

Ibgreen

Bench: Async benchmark tests run very slow

Background: I'm running a benchmark test where I created an async function with a 5 second delay. The benchmark took 178s on average to run. If I lower the delay to 1 second, the benchmark took 30 seconds, when I increase the delay to 10 seconds, the benchmark took 318s.

Question: I'm curious why a 5 second function took 3 minutes to run, or a 10 second function took 5+ minutes to run. Is this a bug or is Bench intended to take this long?

Note: I noticed after further research that runBenchTestForMinimumTimeAsync has a multiplier of 10 by default on line 306 in bench.ts and it cannot be changed. Is there anything that can be done to be able to control that?

Forked On 06 Sep 2022 at 06:46:48

Ibgreen

Without having time to look in detail:

probe runs test cases multiple times to get an average score.

  • For async tests, there should be a flag to run things in parallel to test total throughput, rather than in sequence.

The 10x multiplier may be related to a wish to make the results less flaky (on a mac, timings typically vary ~10-20% between runs due to OS scheduling etc, which makes it hard to make precise decisions based on timings).

However, faster but a bit less precise timings are still preferred in many situations.

Commented On 06 Sep 2022 at 06:46:48
Merge

Ibgreen

fix(tile-converter): types
Forked On 02 Sep 2022 at 02:31:41

Ibgreen

It looks weird to explicitly set these to undefined instead of omitting them. That would normally never happen. Maybe comment on what we are testing?
On 02 Sep 2022 at 02:31:41

Ibgreen

fix(tile-converter): types
Merged On 02 Sep 2022 at 02:32:13

Ibgreen

Commented On 02 Sep 2022 at 02:32:13