Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=196710
Summary: Review Request: coldet - 3D Collision Detection Library Product: Fedora Extras Version: devel Platform: All OS/Version: Linux Status: NEW Severity: normal Priority: normal Component: Package Review AssignedTo: bugzilla-sink@leemhuis.info ReportedBy: j.w.r.degoede@hhs.nl QAContact: fedora-package-review@redhat.com
Spec URL: http://people.atrpms.net/~hdegoede/coldet.spec SRPM URL: http://people.atrpms.net/~hdegoede/coldet-1.1-1.src.rpm Description: This library is an effort to provide a free collision detection library for generic polyhedra. Its purpose is mainly for 3D games where accurate detection is needed between two non-simple objects.
Features: * Works on any model, even polygon soups. * Uses bounding box hierarchies for fast detection. * Uses additional triangle intersection tests for 100% accuracy. * Provides (upon request) exact point of collision, plus the pair of triangles that collided. * Supports timeout setting, to limit detection time. * Model-Model collision test. * Ray-Model collision test. * Segment-Model collision test. * Sphere-Model collision test. * Ray-Sphere and Sphere-Sphere primitive collision tests.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: coldet - 3D Collision Detection Library
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=196710
------- Additional Comments From rc040203@freenet.de 2006-06-26 12:46 EST ------- The *-devel packages installs its headers to /usr/include, resulting into this: /usr/include/coldet.h /usr/include/math3d.h
To me, /usr/include/math3d.h is too general to justify installing it there[1]. I strongly recommend to install the headers into /usr/include/coldet instead.
Also, the package suffers from quite an amount of rather serious GCC warnings (esp. punned pointers), which are not unlikely to break its functionality.
[1] I am working on 3d simulations myself and have several packages providing /usr/include/*/math3d.h installed.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: coldet - 3D Collision Detection Library
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=196710
------- Additional Comments From j.w.r.degoede@hhs.nl 2006-06-26 15:38 EST ------- (In reply to comment #1)
The *-devel packages installs its headers to /usr/include, resulting into this: /usr/include/coldet.h /usr/include/math3d.h
To me, /usr/include/math3d.h is too general to justify installing it there[1]. I strongly recommend to install the headers into /usr/include/coldet instead.
I was thinking this at first too, then I though it would be ok and that if it wouldn't be ok I would hear so during the review :)
Also, the package suffers from quite an amount of rather serious GCC warnings (esp. punned pointers), which are not unlikely to break its functionality.
Huh, quite an amount? on x86_64 devel I count 3 "dereferencing type-punned pointer will break strict-aliasing rules" warnings. I've once spend a days fixing all those warnings in Glide3. But I've given up there are simple to many of these warnings and 99.9% is not a problem. I know howto fix these, but I don't see how this package is any different from all the other packages we have with these kind of warnings.
Here is a new version soon moving the headers to /usr/include/coldet: Spec URL: http://people.atrpms.net/~hdegoede/coldet.spec SRPM URL: http://people.atrpms.net/~hdegoede/coldet-1.1-2.src.rpm
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: coldet - 3D Collision Detection Library
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=196710
------- Additional Comments From rc040203@freenet.de 2006-06-26 15:57 EST ------- (In reply to comment #2)
(In reply to comment #1)
Huh, quite an amount? on x86_64 devel I count 3 "dereferencing type-punned pointer will break strict-aliasing rules" warnings. I've once spend a days fixing all those warnings in Glide3. But I've given up there are simple to many of these warnings
=> Crappy, non portable code.
and 99.9% is not a problem.
They might currently not be a problem, but they are very likely to become in future, once GCC should start using more agressive optimizations.
I know howto fix these, but I
don't see how this package is any different from all the other packages we have with these kind of warnings.
The only difference is this package consisting of very few source files.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: coldet - 3D Collision Detection Library
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=196710
j.w.r.degoede@hhs.nl changed:
What |Removed |Added ---------------------------------------------------------------------------- OtherBugsDependingO| |196744 nThis| |
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: coldet - 3D Collision Detection Library
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=196710
------- Additional Comments From rc040203@freenet.de 2006-06-27 05:32 EST ------- (In reply to comment #2) Hans, I think, I need to clarify my comment:
I would approve this package if you install the headers to /usr/include/coldet.
My remark on the "punned pointers" was not meant to be blocker to the review. It was meant as a "BIG FAT WARNING" to you that you will not unlikely be facing "weird runtime errors" related to this package in future.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: coldet - 3D Collision Detection Library
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=196710
wart@kobold.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED AssignedTo|bugzilla-sink@leemhuis.info |wart@kobold.org OtherBugsDependingO|163776 |163778 nThis| |
------- Additional Comments From wart@kobold.org 2006-07-06 19:19 EST ------- MUST ==== * Package/spec named appropriately * Source matches upstream: 26c2db12ec5ad2d7a0b1d0fe2597ed4a coldet_11.zip * LGPL license ok, license file included * rpmlint output clean * spec file legible and in Am. English * Builds ok in mock on FC4, FC5, and FC6 for both i386 and x86_64 * No excessive BR: (no BR: at all!) * ldconfig called in %post/%postud as needed * headers and unversioned .so properly located in -devel subpackage * Owns all directories that it creates; doesn't own directories that it should not. * No locales * No .desktop file needed * Not relocatable * No duplicate %files * File permissions look ok * No libtool archives
MUSTFIX ======= * -devel subpackage missing the -%{release} component of e-v-r when requiring the base package.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: coldet - 3D Collision Detection Library
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=196710
------- Additional Comments From j.w.r.degoede@hhs.nl 2006-07-07 07:24 EST ------- (In reply to comment #5)
MUSTFIX
- -devel subpackage missing the -%{release} component of e-v-r when requiring the base package.
Fixed, new version is here: Spec URL: http://people.atrpms.net/~hdegoede/coldet.spec SRPM URL: http://people.atrpms.net/~hdegoede/coldet-1.1-2.src.rpm
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: coldet - 3D Collision Detection Library
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=196710
------- Additional Comments From j.w.r.degoede@hhs.nl 2006-07-07 07:24 EST ------- Correction that should be: Spec URL: http://people.atrpms.net/~hdegoede/coldet.spec SRPM URL: http://people.atrpms.net/~hdegoede/coldet-1.1-3.src.rpm
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: coldet - 3D Collision Detection Library
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=196710
wart@kobold.org changed:
What |Removed |Added ---------------------------------------------------------------------------- OtherBugsDependingO|163778 |163779 nThis| |
------- Additional Comments From wart@kobold.org 2006-07-07 11:07 EST ------- All MUSTFIX items addressed.
APPROVED
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: coldet - 3D Collision Detection Library
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=196710
j.w.r.degoede@hhs.nl changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution| |NEXTRELEASE
------- Additional Comments From j.w.r.degoede@hhs.nl 2006-07-07 14:35 EST ------- Thanks! Imported & build, closing.
package-review@lists.fedoraproject.org
-
Red Hat Bugzilla