Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
Summary: Review Request: nvidia-texture-tools - Collection of image processing and texture manipulation tools
https://bugzilla.redhat.com/show_bug.cgi?id=823096
Summary: Review Request: nvidia-texture-tools - Collection of image processing and texture manipulation tools Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: medium Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: paulo.cesar.pereira.de.andrade@gmail.com QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, package-review@lists.fedoraproject.org Classification: Fedora Story Points: --- Type: --- Regression: --- Mount Type: --- Documentation: ---
Spec URL: http://fedorapeople.org/~pcpa/nvidia-texture-tools.spec SRPM URL: http://fedorapeople.org/~pcpa/nvidia-texture-tools-2.0.8-1.fc18.src.rpm Description: The NVIDIA Texture Tools is a collection of image processing and texture manipulation tools, designed to be integrated in game tools and asset conditioning pipelines.
The primary features of the library are mipmap and normal map generation, format conversion and DXT compression.
DXT compression is based on Simon Brown's squish library. The library also contains an alternative GPU-accelerated compressor that uses CUDA and is one order of magnitude faster.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=823096
--- Comment #1 from pcpa paulo.cesar.pereira.de.andrade@gmail.com 2012-05-18 23:15:38 EDT --- koji --scratch build pass on i686 and x86_64.
Patches were reported upstream at http://code.google.com/p/nvidia-texture-tools/issues/detail?id=175
This package will be required to build 0ad (#818401), as I am reworking it to not build an internal copy of an older version of nvidia-texture-tools.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=823096
pcpa paulo.cesar.pereira.de.andrade@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |818401
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=823096
Peter Lemenkov lemenkov@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |182235(FE-Legal)
--- Comment #2 from Peter Lemenkov lemenkov@gmail.com 2012-05-19 00:23:13 EDT --- Lifting FE-LEGAL due to possible issues with S3TC
http://code.google.com/p/nvidia-texture-tools/wiki/FAQ
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=823096
--- Comment #3 from pcpa paulo.cesar.pereira.de.andrade@gmail.com 2012-05-19 00:51:00 EDT --- Thanks for noticing it.
A grep -ri s3tc shows only references to it in src/nvtt/squish/texture_compression_s3tc.txt
otherwise, the only other reference is in src/nvtt/CompressDXT.cpp
#if defined(HAVE_S3QUANT) #include "s3tc/s3_quant.h" #endif
and HAVE_S3QUANT is not defined.
and the actual code, in the same file is also inside an inclusion guard:
#if defined(HAVE_S3QUANT) void nv::s3CompressDXT1(const Image * image, const nvtt::OutputOptions::Private & outputOptions) { ... } #endif
Actually, that same source also has another
#if defined(HAVE_ATITC) #include "atitc/ATI_Compress.h" #endif
and a void nv::atiCompressDXT1 function compiled if HAVE_ATITC is defined.
So, from my understanding, there is no binary code using it, neither exposing interfaces to it.
But there is source code that is not compiled, that may interface those if the devel/runtime is detected at build time.
https://bugzilla.redhat.com/show_bug.cgi?id=823096
--- Comment #4 from Adam Jackson ajax@redhat.com --- (In reply to comment #3)
Thanks for noticing it.
A grep -ri s3tc shows only references to it in src/nvtt/squish/texture_compression_s3tc.txt
Simply searching for s3tc is not enough. There are multiple variants of S3TC, named DXT[1-5]. Which I definitely see code for in here. I don't think this is something Fedora can ship.
https://bugzilla.redhat.com/show_bug.cgi?id=823096
--- Comment #5 from pcpa paulo.cesar.pereira.de.andrade@gmail.com --- (In reply to comment #4)
(In reply to comment #3)
Thanks for noticing it.
A grep -ri s3tc shows only references to it in src/nvtt/squish/texture_compression_s3tc.txt
Simply searching for s3tc is not enough. There are multiple variants of S3TC, named DXT[1-5]. Which I definitely see code for in here. I don't think this is something Fedora can ship.
I was under the impression that it did only interface to s3tc, but it actually embeds an open source implementation, that was already blocked at https://bugzilla.redhat.com/show_bug.cgi?id=407561
For more information, start of thread in debian, http://lists.debian.org/debian-mentors/2010/09/msg00080.html
There is a comment about DeVIL (also in fedora) http://lists.debian.org/debian-mentors/2010/09/msg00129.html but from what I could read in DeVIL sources, it does not include an implementation, and only attempts to get pointers to it from the libGL.
And apparently debian decision was to allow it due to not enforced patent, but that should be an option for debian and not for Fedora/RedHat http://lists.debian.org/debian-mentors/2010/09/msg00130.html
Nevertheless, looking more closely at 0ad build options, I see the default is to build a bundled nvtt, or respect --with-system-nvtt, but there is yet another --without-nvtt that I suppose is an alternative to either textures already compressed in 0ad-data or just not compressing it.
Reading the sources, --without-nvtt defines CONFIG2_NVTT to 0, and only source to use it is <0ad-src>/source/graphics/TextureConverter.cpp with functions that can be called from <0ad-src>/source/graphics/TextureManager.cpp
I will try a --without-nvtt build later, and see the results, e.g. if not filling my disk, or console with logs:
-%<- bool CTextureConverter::ConvertTexture(const CTexturePtr& texture, const VfsPath& src, const VfsPath& dest, const Settings& settings) { /* load image from vfs, checks, etc */ #if CONFIG2_NVTT ... request->compressionOptions.setQuality(m_HighQuality ? nvtt::Quality_Production : nvtt::Quality_Fastest);
// TODO: normal maps, gamma, etc
// Load the texture data request->inputOptions.setTextureLayout(nvtt::TextureType_2D, tex.w, tex.h); request->inputOptions.setMipmapData(tex_get_data(&tex), tex.w, tex.h);
// NVTT copies the texture data so we can free it now tex_free(&tex);
pthread_mutex_lock(&m_WorkerMutex); m_RequestQueue.push_back(request); pthread_mutex_unlock(&m_WorkerMutex);
// Wake up the worker thread SDL_SemPost(m_WorkerSem);
return true;
#else LOGERROR(L"Failed to convert texture "%ls" (NVTT not available)", src.string().c_str()); return false; #endif } -%<-
There is an upstream page about the issue at http://trac.wildfiregames.com/wiki/CompressedTextures
https://bugzilla.redhat.com/show_bug.cgi?id=823096
--- Comment #6 from pcpa paulo.cesar.pereira.de.andrade@gmail.com --- I added a comment at http://trac.wildfiregames.com/ticket/1427 and tested a build with that patch and the package appears to work. Major change was to need to disable %check in the spec due to missing nvtt.
https://bugzilla.redhat.com/show_bug.cgi?id=823096
pcpa paulo.cesar.pereira.de.andrade@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |CLOSED Resolution|--- |WONTFIX Last Closed| |2012-07-13 10:18:58
--- Comment #7 from pcpa paulo.cesar.pereira.de.andrade@gmail.com --- The package builds code under a patent and is not eligible for fedora. I made a review request for the same package in rpmfusion, but there is a workaround to build 0ad without nvtt.
https://bugzilla.redhat.com/show_bug.cgi?id=823096
pcpa paulo.cesar.pereira.de.andrade@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks|818401 |
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=823096
Tom "spot" Callaway tcallawa@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|CLOSED |ASSIGNED CC| |tcallawa@redhat.com Blocks|182235 (FE-Legal) | Resolution|WONTFIX |--- Keywords| |Reopened
--- Comment #8 from Tom "spot" Callaway tcallawa@redhat.com --- Upon further review, this package is no longer blocked on FE-Legal. It does contain an S3TC implementation, but the patent license provided by NVIDIA has been determined to be sufficient. Please do not take the S3TC implementation from this codebase and copy it into any other code, because that would not be covered under the terms of the NVIDIA patent license (and not acceptable for Fedora).
Lifting FE-Legal
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=823096
Tom "spot" Callaway tcallawa@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-review+
--- Comment #9 from Tom "spot" Callaway tcallawa@redhat.com --- == Review ==
Good:
- rpmlint checks return: nvidia-texture-tools.x86_64: W: spelling-error %description -l en_US mipmap -> mishap nvidia-texture-tools.x86_64: W: shared-lib-calls-exit /usr/lib64/libnvcore.so.2.0.8 exit@GLIBC_2.2.5 nvidia-texture-tools.src: W: spelling-error %description -l en_US mipmap -> mishap
All safe to ignore
- package meets naming guidelines - package meets packaging guidelines - license (MIT) OK, text in %doc, matches source - spec file legible, in am. english - source matches upstream (6b3c83500b420ee976f61eeae16e5727e2401e133f543baeac76c66c381eed2e) - package compiles on devel (x86_64) - no missing BR - no unnecessary BR - no locales - not relocatable - owns all directories that it creates - no duplicate files - permissions ok - macro use consistent - code, not content - no need for -docs - nothing in %doc affects runtime - no need for .desktop file - devel package ok - no .la files - post/postun ldconfig ok - devel requires base package n-v-r
MINOR ISSUES:
* The source url is wrong, should be: http://nvidia-texture-tools.googlecode.com/files/%%7Bname%7D-%%7Bversion%7D-...
* I don't see the value in a separate "progs" subpackage (it is only 41K). I strongly suggest just combining it into the main package (especially since the naming "nvidia-texture-tools" implies the presence of tools), but I leave that to your discretion.
Please fix the source URL (and dropping the "progs" subpackage if you choose to do so) before committing to Fedora, but this package is otherwise very well done.
APPROVED.
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=823096
Tom "spot" Callaway tcallawa@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Assignee|nobody@fedoraproject.org |tcallawa@redhat.com
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=823096
--- Comment #10 from Paulo Andrade paulo.cesar.pereira.de.andrade@gmail.com --- (In reply to comment #8)
Upon further review, this package is no longer blocked on FE-Legal. It does contain an S3TC implementation, but the patent license provided by NVIDIA has been determined to be sufficient. Please do not take the S3TC implementation from this codebase and copy it into any other code, because that would not be covered under the terms of the NVIDIA patent license (and not acceptable for Fedora).
Lifting FE-Legal
Many thanks for looking back at this issue and reviewing it!
I updated the package before requesting scm and should now match fully your recommendations:
- Correct source url (#823096). - No need for a -progs subpackage (#823096).
Spec URL: http://fedorapeople.org/~pcpa/nvidia-texture-tools.spec SRPM URL: http://fedorapeople.org/~pcpa/nvidia-texture-tools-2.0.8-3.fc19.src.rpm
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=823096
Paulo Andrade paulo.cesar.pereira.de.andrade@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-cvs?
--- Comment #11 from Paulo Andrade paulo.cesar.pereira.de.andrade@gmail.com --- New Package SCM Request ======================= Package Name: nvidia-texture-tools Short Description: Collection of image processing and texture manipulation tools Owners: pcpa Branches: f16 f17 f18 InitialCC:
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=823096
--- Comment #12 from Jon Ciesla limburgher@gmail.com --- Git done (by process-git-requests).
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=823096
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |MODIFIED
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=823096
--- Comment #13 from Fedora Update System updates@fedoraproject.org --- 0ad-0.0.12-2.fc16,0ad-data-0.0.12-1.fc16,nvidia-texture-tools-2.0.8-3.fc16 has been submitted as an update for Fedora 16. https://admin.fedoraproject.org/updates/0ad-0.0.12-2.fc16,0ad-data-0.0.12-1....
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=823096
--- Comment #14 from Fedora Update System updates@fedoraproject.org --- 0ad-0.0.12-2.fc17,0ad-data-0.0.12-1.fc17,nvidia-texture-tools-2.0.8-3.fc17 has been submitted as an update for Fedora 17. https://admin.fedoraproject.org/updates/0ad-0.0.12-2.fc17,0ad-data-0.0.12-1....
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=823096
--- Comment #15 from Fedora Update System updates@fedoraproject.org --- 0ad-0.0.12-2.fc18,0ad-data-0.0.12-1.fc18,nvidia-texture-tools-2.0.8-3.fc18 has been submitted as an update for Fedora 18. https://admin.fedoraproject.org/updates/0ad-0.0.12-2.fc18,0ad-data-0.0.12-1....
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=823096
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |ON_QA
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=823096
--- Comment #16 from Fedora Update System updates@fedoraproject.org --- Package 0ad-0.0.12-2.fc16, 0ad-data-0.0.12-1.fc16, nvidia-texture-tools-2.0.8-3.fc16: * should fix your issue, * was pushed to the Fedora 16 testing repository, * should be available at your local mirror within two days. Update it with: # su -c 'yum update --enablerepo=updates-testing 0ad-0.0.12-2.fc16 0ad-data-0.0.12-1.fc16 nvidia-texture-tools-2.0.8-3.fc16' as soon as you are able to. Please go to the following url: https://admin.fedoraproject.org/updates/FEDORA-2012-20773/0ad-0.0.12-2.fc16,... then log in and leave karma (feedback).
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=823096
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ON_QA |CLOSED Resolution|--- |CURRENTRELEASE Last Closed|2012-07-13 10:18:58 |2012-12-29 01:26:19
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=823096
--- Comment #17 from Fedora Update System updates@fedoraproject.org --- 0ad-0.0.12-2.fc17, 0ad-data-0.0.12-1.fc17, nvidia-texture-tools-2.0.8-3.fc17 has been pushed to the Fedora 17 stable repository. If problems still persist, please make note of it in this bug report.
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=823096
--- Comment #18 from Fedora Update System updates@fedoraproject.org --- 0ad-0.0.12-2.fc16, 0ad-data-0.0.12-1.fc16, nvidia-texture-tools-2.0.8-3.fc16 has been pushed to the Fedora 16 stable repository. If problems still persist, please make note of it in this bug report.
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=823096
--- Comment #19 from Fedora Update System updates@fedoraproject.org --- 0ad-0.0.12-2.fc18, 0ad-data-0.0.12-1.fc18, nvidia-texture-tools-2.0.8-3.fc18 has been pushed to the Fedora 18 stable repository. If problems still persist, please make note of it in this bug report.
package-review@lists.fedoraproject.org