Bug 823096

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.

Lifting FE-LEGAL due to possible issues with S3TC

http://code.google.com/p/nvidia-texture-tools/wiki/FAQ

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.

(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.

(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

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.

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.

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

== 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.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/%{name}-%{version}-1.tar.gz

* 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.

(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

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:

Git done (by process-git-requests).

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.fc16,nvidia-texture-tools-2.0.8-3.fc16

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.fc17,nvidia-texture-tools-2.0.8-3.fc17

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.fc18,nvidia-texture-tools-2.0.8-3.fc18

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,0ad-data-0.0.12-1.fc16,nvidia-texture-tools-2.0.8-3.fc16
then log in and leave karma (feedback).

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.

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.

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.