joostn / OpenJsCad

3D solid CAD using only Javascript
315 stars 128 forks source link

Fixed issue 19 #20

Closed tedbeer closed 11 years ago

tedbeer commented 11 years ago

With this fix I tested all examples from the repository.

joostn commented 11 years ago

Hi Ed,

I've applied you patch but the problem is still not fixed completely: Try this:

function main(params)
{
  var plate2d=CAG.rectangle({radius: 50})
               .subtract(CAG.rectangle({radius: 30}));
  var extruded = plate2d.extrude({offset: [0,0,4]});
  return extruded;
}

This should give a cube with a rectangular cutout but it looks like the inside side faces have the wrong orientation.

Note that 'return plate2d' will also show the problem since extrude() is used internally by openjscad to display CAGs.

tedbeer commented 11 years ago

Hi!

I will have a look. I can restore the old code for requests without transformation and use a new one only for calls with transformation. In this case it will be fully compatible with all old scenarios. And may have some limits in a case of custom transformation. When we collect enough objects for testing we can switch to this algorithm more safely.

Best regards, Eduard.

On Tue, Jan 15, 2013 at 11:31 AM, joostn notifications@github.com wrote:

Hi Ed,

I've applied you patch but the problem is still not fixed completely: Try this:

function main(params){ var plate2d=CAG.rectangle({radius: 50}) .subtract(CAG.rectangle({radius: 30})); var extruded = plate2d.extrude({offset: [0,0,4]}); return extruded;}

This should give a cube with a rectangular cutout but it looks like the inside side faces have the wrong orientation.

Note that 'return plate2d' will also show the problem since extrude() is used internally by openjscad to display CAGs.

— Reply to this email directly or view it on GitHubhttps://github.com/joostn/OpenJsCad/pull/20#issuecomment-12260994.

joostn commented 11 years ago

I've quickly tried to fix your code but it's too complex for just a quick look. For now I've renamed your version to extrudeExperimental() and re-added the former code

tedbeer commented 11 years ago

The code is pretty simple if I explain :) At least the idea is simple. We can meet :) I work in Amsterdam.

On Tue, Jan 15, 2013 at 2:48 PM, joostn notifications@github.com wrote:

I've quickly tried to fix your code but it's too complex for just a quick look. For now I've renamed your version to extrudeExperimental() and re-added the former code

— Reply to this email directly or view it on GitHubhttps://github.com/joostn/OpenJsCad/pull/20#issuecomment-12267330.

joostn commented 11 years ago

Yes it should be easy, just ensure that the vertex ordering is correct in all cases. But I have zero time to spend on openjscad at the moment..

tedbeer commented 11 years ago

Let's leave it as it currently is until I'll implement some test cases and we have decent test coverage. I postpone the story a bit. I've just built 3D printer and need to fine tune it.

tedbeer commented 11 years ago

I created a test brunch in my repository - https://github.com/tedbeer/OpenJsCad/tree/test-jscoverage csg.js is split into modules to make testing easier. I added you as a collaborator so you can push directly(I hope, never tried, I don't like git). To get JS coverage report locally you need to install small server - http://siliconforks.com/jscoverage/ Tests don't check if everything work correctly they just try to execute as much code as possible. You can check the current state and how JS coverage report looks like here(I started from the bottom) - http://ed.tedbeer.net/jsc/