Makman2 / CE3D

A terminal 3D engine
GNU General Public License v3.0
4 stars 0 forks source link

Implement line-drawing in console-renderer #160

Closed Makman2 closed 9 years ago

Makman2 commented 9 years ago

The console renderer now supports drawing lines. This pull requests also corrects some aesthetic failures.

Makman2 commented 9 years ago

Next time triangle drawing will come. I also think of an hybrid mode where the triangle edges are drawn like lines and the inside is clear or filled with a specific char, i. e.

__________ 
\########/
 \######/
  \####/
   \##/
    \/
Makman2 commented 9 years ago

Do not merge, forgot to implement the z-buffer.

sils commented 9 years ago

I'm full of suspense. Commits look good on first sight. :)

Makman2 commented 9 years ago

Found also an error in the algorithm. I needed to rewrite x1,y1 ... to p1[0] and so on, and I did a mistake :O So I need to rebase anyway :P

Makman2 commented 9 years ago

So z-buffer still missing... will come.

Makman2 commented 9 years ago

Done. Build is okay, the algorithm should work (but I'm not sure with the z-coordinate-calculation, and this component is really hard to test, but this should also work :D).

Free for merge

sils commented 9 years ago

okay, my experience is: everything is testable. Its just a bit difficult sometimes.

Makman2 commented 9 years ago

That's the point. And I think now we come to try and error :O

Makman2 commented 9 years ago

Removed unneeded overhead.

Makman2 commented 9 years ago

Okay everything settled I think, will begin to fix :)

PS: Meantime I already wrote the the triangle drawing algorith, so it won't take much time after this pull request to implement this.

Makman2 commented 9 years ago
reword a3a3681 renderer: Correct description failure
edit 04b828c renderer: Rename functions
edit 516e41b renderer/console: Implement line drawing
fixup 68280d1 renderer/console: Remove duplicate calculation
pick 47f9c2b renderer/console: Codestyle improvements
edit 27135e9 renderer/console: Let DrawPoint() update z-buffer
pick a747d93 renderer/console: DrawLine() uses now z-buffer

Split up commit `04b828c: Rename functions" into description changing and renaming commit. This is quite an extreme rebase^^

Makman2 commented 9 years ago

Ah there comes me something into mind: Do we want to use asserts in the drawing function? I think about to check the dimension of the input vectors. For example for DrawPoint() and it's Vector const& point parameter, I would write an assert like this:

BOOST_ASSERT_MSG(point.size() > 2, "Point parameter dimension bigger than supported."
                                   "Higher dimensions ignored.")
sils commented 9 years ago

I wouldnt use an assert since the function also works on those vectors but maybe issue a warning or so. We'll need a logging system anyway, probablyt theres something for this in boost?

Makman2 commented 9 years ago

Seems so: Boost.Log v2

Makman2 commented 9 years ago

Make issue for that?

sils commented 9 years ago

make issue for that and put todo in your file!

2014-12-13 20:00 GMT+01:00 Makman2 notifications@github.com:

Make issue for that?

Reply to this email directly or view it on GitHub https://github.com/Makman2/CE3D/pull/160#issuecomment-66886945.

Makman2 commented 9 years ago

See #163.

Makman2 commented 9 years ago

Okay everything settled so I can rebase again?

sils commented 9 years ago

go ahead

2014-12-13 20:08 GMT+01:00 Makman2 notifications@github.com:

Okay everything settled so I can rebase again?

Reply to this email directly or view it on GitHub https://github.com/Makman2/CE3D/pull/160#issuecomment-66887185.

Makman2 commented 9 years ago
edit 77b1b8a renderer/console: Implement line drawing
pick 7cff06a renderer/console: Codestyle improvements
pick 181e81a renderer/console: Let DrawPoint() update z-buffer
reword a4deb51 renderer/console: DrawLine() uses now z-buffer
Makman2 commented 9 years ago

Tests do come later, that's quite a bit coding work.

sils commented 9 years ago

go ahead, rebase

Makman2 commented 9 years ago
edit a34b037 renderer/console: Implement line drawing
pick 1d3f4bc renderer/console: Codestyle improvements
pick 7e1c0a8 renderer/console: Let DrawPoint() update z-buffer
pick bbcaa6b renderer/console: Use Z-Buffer in DrawLine()
pick 814da3a renderer/console: Insert TODOs for logging
Makman2 commented 9 years ago

Okay if you've nothing to say on this branch I'll make the NaN catch and rebase again.

sils commented 9 years ago

anything missing?

Makman2 commented 9 years ago

not to fogert to insert a TODO for Boost.Log... EDIT: Never mind... already done...

Makman2 commented 9 years ago

Okay everything's settled, going to rebase...

Makman2 commented 9 years ago

Forgot to copy the right rebase script... anyway not so important because some edits were involved^^

Makman2 commented 9 years ago

Rebased^^

sils commented 9 years ago

ok, everything should be mergeable by now

sils commented 9 years ago

I'll let you do the merge, you should know this better.

Makman2 commented 9 years ago

I want to test the Z-buffer algorithm manually first, after that I'm going to merge^^

sils commented 9 years ago

good, see? I missed that ;)

2015-01-15 0:05 GMT+01:00 Makman2 notifications@github.com:

I want to test the Z-buffer algorithm manually first, after that I'm going to merge^^

Reply to this email directly or view it on GitHub https://github.com/Makman2/CE3D/pull/160#issuecomment-70010892.

Makman2 commented 9 years ago

It was a good idea to test it manually... I did some quite big mistakes... So that will give a rebase, but you only need to ack the commit that involves the line drawing again^^

Makman2 commented 9 years ago

Better idea: I'll push a fixup commit that you can ack, if you acked it then I'll rebase and merge^^

Makman2 commented 9 years ago

Now it's working.

Makman2 commented 9 years ago
pick 12fe596 renderer/console: Use Z-Buffer in DrawLine()
fixup 9673929 FIXUP WITH 12fe596: Fix z-buffer usage
fixup 7c6260d FIXUP WITH 12fe596: Resort z-buffer variables
pick 96890dd codestyle: Add advised style note
Makman2 commented 9 years ago

Okay that's it, merging.

sils commented 9 years ago

Cool! Do you have a master plan on how we get CE3D into a usable state? What's missing? That is long overdue IMO. Am 18.01.2015 18:57 schrieb "Makman2" notifications@github.com:

Merged #160 https://github.com/Makman2/CE3D/pull/160.

Reply to this email directly or view it on GitHub https://github.com/Makman2/CE3D/pull/160#event-220666456.

Makman2 commented 9 years ago

De facto you can use it, but the Orthogonal Projection is missing some features to make it work in all dimensions (the world's up thing, but I have problems to bring it together with the lookat vector). If this is done we can announce v0.1 :D

sils commented 9 years ago

Then you should update instructions on how to build and execute the thing plus a mini example application. You can probably extend the one I wrote back then.

2015-01-18 21:01 GMT+01:00 Makman2 notifications@github.com:

De facto you can use it, but the Orthogonal Projection is missing some features to make it work in all dimensions (the world's up thing, but I have problems to bringt it together with the lookat vector). If this is done we can announce v0.1 :D

Reply to this email directly or view it on GitHub https://github.com/Makman2/CE3D/pull/160#issuecomment-70423123.

Makman2 commented 9 years ago

Good thing, will do that