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

Process and Layer imagery/elevation #1068

Merged
merged 7 commits into from
Mar 13, 2019
Merged

Conversation

gchoqueux
Copy link
Contributor

Description

Work on process and Layer imagery/elevation:

  • Move (see delete) parts of the code into more appropriate classes or clearer functions.
  • Extract common methods.
  • Fix

the problem comes from LayeredMaterialNodeProcessing.updateLayeredMaterialNodeImagery.
No texture is provided for a tile, if only one of them is outside the source extent.

Visit PR iTowns#1067 iTowns#1067 to follow the issue.
Copy link
Contributor

@zarov zarov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this first cleaning !

@@ -54,6 +43,36 @@ function refinementCommandCancellationFn(cmd) {
return !cmd.requester.material.visible;
}

function handlingError(err, node, layer, targetLevel, view) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't this be moved to TileMesh or in a ProcessHelper file along with buildCommand below ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I move handlingError but not buildCommand is specify for each process

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

src/Converter/convertToTile.js Show resolved Hide resolved
…ssing.

extract common functions in layered material processing and feature processing:
   * The target is principally to highlight the common parties and simplify the refactoring in progress.

other:
   * remove useless test command parameter.
Copy link
Contributor

@zarov zarov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@gchoqueux gchoqueux merged commit 7475a3f into iTowns:master Mar 13, 2019
@gchoqueux gchoqueux deleted the process_imagery branch March 13, 2019 09:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants