Russian Qt Forum

Добрый день

Вот работаю с библиотекой, приходится вникать/разбираться

C++ (Qt)
void SkMotion::apply ( float t,  
	float* buffer, SrBuffer<int>* map_p,
	InterpType itype, int* lastframe, bool isAdditive, SBRetarget* retarget )
{
	// new version of apply skMotion
	int fsize=_frames.size();
	if ( fsize<=0 )
		return;
	if ( t!= 0.f && t<=_frames[0].keytime )	{
#if DEBUG_T
		LOG("SkMotion::apply NOTICE: t=%.16f < f[0]:%.16f \n", t, _frames[0].keytime );
#endif
		return;
	}
 
	if ( itype==CubicSpline )
		t = _cubic ( t, _frames[0].keytime, _frames[_frames.size() - 1].keytime );
 
#if DEBUG_T
	if ( t<_frames[0].keytime )	{
		LOG("SkMotion::apply ERR: cubic t=%.16f < f[0]:%.16f \n", t, _frames[0].keytime );
	}
#endif
 
	// optimize keytime search for sequenced calls to apply
	int fini=0;
	if ( lastframe )
		_last_apply_frame = *lastframe;
	if ( _last_apply_frame>0 && _last_apply_frame<fsize ) {
		if ( t>_frames[_last_apply_frame].keytime )
			fini=_last_apply_frame+1;
	}
 
	int f;
	for ( f=fini; f<fsize; f++ ) {
		if ( t<_frames[f].keytime )
			break;
	}
 
	if ( f==_frames.size() ) {
		// Apply last frame
		//apply_frame( f-1, buffer, map_p );
		//return;
		t = _frames[_frames.size()-1].keytime;
		f = _frames.size()-1;
	}	
 
	if (_frames.size() > 1)
	{
		f--;
	}	
	_last_apply_frame = f;
	if ( lastframe )
		*lastframe = _last_apply_frame;
	int nxtFrame = _frames.size() > 1 ? f+1 : f;
 
	float* fp1 = _frames[f].posture;
	float* fp2 = _frames[nxtFrame].posture;
	// convert t to [0,1] according to the keytimes:
	t = _frames.size() > 1 ? (t-_frames[f].keytime) / (_frames[f+1].keytime-_frames[f].keytime) : 0.f;
 
#if DEBUG_T
	if ( t<0.0 )	{
		LOG("SkMotion::apply ERR: mapped t=%.16f < 0.0 \n", t );
	}
#endif
 
	//sr_out<<"t: "<<t<<" frames: "<<f<<srspc<<(f+1)<<"\n";
	int num;
	int csize = _channels.size();
 
 
	float outValue[4], origValue[4], newValue[4]; // at most 4 
	// Apply to float* buffer
	float* v = NULL;
	int sum = 0;		
	for ( int i=0; i<csize; i++ ) {
	// Channel size
		num = _channels[i].size();
		// Find mapped buffer location
		int index = i;
 
		if (buffer)
		{
			if (map_p)
			{
				index = map_p->get( i );
				v = buffer + index;
			}
			else
			{
				v = buffer + sum;
			}
			for (int x=0;x<num;x++)
				origValue[x] = v[x];				
		}
		else if (_channels[i].joint)
		{
			_channels[i].get(origValue);						
		}
		else 
		{
			index = -1;
		}
 
		_channels[i].interp ( newValue, fp1, fp2, t );
		if( index >= 0 ) {
			if (_channels[i].type == SkChannel::Quat) // rotation channel
			{				
				SrQuat orig(origValue[0], origValue[1], origValue[2], origValue[3]);
				SrQuat add(newValue[0], newValue[1], newValue[2], newValue[3]);
				SrQuat final = isAdditive ? orig*add : add; 	
				if (retarget)
					final = retarget->applyRetargetJointRotation(_channels.mappedName(i),final);
				for (int x = 0; x < 4; x++)
					outValue[x] = final.getData(x);
			}
			else if (num <= 3) // positional channel
			{				
				for (int x = 0; x < num; x++)
				{
					outValue[x] = isAdditive? origValue[x]+newValue[x] : newValue[x];
					if (retarget)
						outValue[x] = retarget->applyRetargetJointTranslation(_channels.mappedName(i),outValue[x]);
				}
			}
			else
			{
				// more that 3 channels but not a quaternion? Don't add data, just replace it...
				for (int x = 0; x < num; x++)
				{
					outValue[x] = newValue[x];
					if (retarget)
						outValue[x] = retarget->applyRetargetJointTranslation(_channels.mappedName(i),outValue[x]);
				}
			}
			if (buffer)
			{
				for (int x = 0; x < num; x++)
					v[x] = outValue[x];
			}
			else
			{
				_channels[i].set(outValue);
			}
		}	
		// Increment frame data pointers
		fp1+=num; fp2+=num; sum+=num;
	}			
}
 

Нет, конечно а проекте есть классы, виртуалы, std, и даже немного темплейтов и буста. Но все это как-то "сбоку-припеку", основная/подавляющая масса кода как выше, этим делается содержательная часть. И вот как-то здесь я не вижу ну ровным счетом ничего что часто считается "грамотностью" :)

Может как-то "модернизировать"? Ну и что я должен делать - насильно втыкать std/boost (как Хрущев кукурузу)? А смысл? В общем

Ну единственно что понятно - чегойто делает с фреймами, видимо компилит ролик непонятный.

А по вопросу - нормальный код, но явно прослеживает си стайл в работе с переменными и малая информативность названий типо f, t, fsize, csize, v.
Так же часть кода завязана, видимо, на поля класса.
Поэтому чтобы улучшить, оптимизировать, сделать более грамотнее - нужно вообще понять что эта фигня делает в комплексе, как применяется,  в чём неудобство. Хотя - да что я вам рассказываю, вы и сами знаете. Нужно сначала изучить, потом улучшать.

Если код работает, то лучше его не трогать.
Если не нравится способы вызовов методов или набор сущностей, то лучше написать адаптер/обертку - свой API поверх чужой библиотеки.
Если просто сам код не нравится, тогда придется все переписывать самостоятельно :-\, как показывает практика - практически с нуля.

Первый (скромный) успех - Ваш пост содержит 1 содержательную фразу  :)

Неверно работает если additive = true. Хотя возможно я не до конца разобрался

Там .a файл (компилю статычно) под 400 метров (debug) - это ж сколько "оборачивать" надо?

При таком объеме нереально. Нравится или нет - ну от стиля/форматирования написания я не в восторге. А по существу - по-моему вполне норм, разобраться и понять можно. В связи с этим хотелось бы спросить у "воротящих носик" - а что, собсно, так уж плохо? Нет "чего-то модного/крутого"? Так чего? И нужно ли оно здесь?

Самодокументирование ужасное :D

Не трогайте чужой код. Исправьте ошибку, а всё остальное оставьте. Не в ваших силах изменить мир :D

Оборачивать нужно только то, что необходимо в использовании, в понятные для себя методы. Глядишь потом и собственную реализацию вместо этой подставить).
Стиль и форматирование - для этого есть Beautifying Source Code http://doc.qt.io/qtcreator/creator-beautifier.html (http://doc.qt.io/qtcreator/creator-beautifier.html) и инструменты для рефакторинга. Я частенько использую для анализа кода, даже если только для себя).

Не, рекомендации нормальные, хорошие, вряд ли кто-то с ними будет спорить. Но я хотел поговорить о другом.

В принципе этот кусочек кода как бы "окно в параллельный мир" программирования. Конечно правила С++ и там роялят, но совсем иначе. От владения std, boost, и.т.д. здесь толку немного, даже скромных зачаточных знаний хватит. Ну а тонна заученных Qt классов вообще бесполезна. Решающее значение имеют алгоритмы и знание предметной части, при этом решаемые задачи могут быть отнюдь не просты.

И такая ситуация всякий раз когда связываюсь с приличной либой. Напр скоро придется опять лезть в FBX - либа совершенно другого плана, но и там "запас популярных знаний" вообще-то бесполезен  :)

Чисто внешне код страшноват, смесь С и С++.
Названия функций странные - interp(), а рядом applyRetargetJointRotation().
Переменные типа _frames и т.д. тоже моветон, имхо.
Дуст тут, конечно, не нужен.
Оно выглядит достаточно некрасиво и без дуста:)

Ну вообще-то стандартное название линейной интерполяции "lerp" (никак не лучше). С подчеркиванием у него имена членов класса. Я тоже не в восторге от таких имен, но они вполне в рамках допустимого, красиво-некрасиво = субъективное мнение.

Гораздо более "пугающим" выглядит то что код не вертится вокруг каких-то буквариных классов, а делает все сам  :)

Согласен, это может позже вылезти боком, поскольку "велосипедный" код, как правило, потенциально менее надежен, в отличие от "вылизанных годами и сединами" библиотек.

С другой стороны, возможно, это уникальный алгоритм, который нигде ранее не встречался :)

а что плохого в членах класса, начинающихся с _ ?

Да собсно ничего, дело вкуса

А что он делает? Что можно понять по этому кусочку? Конечно разобраться полностью - нужно куда больше исходников и времени, я об этом и не спрашиваю. А что можно сказать "навскидку"?

А хз что он делает. Похож на студенческий говнокод из лабы первокурсника.

Автор не понимает, что такое локальные параметры (передается float t в качестве аргумента, потом этот же t меняется по коду - фууу...)

Автор не знает о существовании оператора switch (проверки типов через if).

В общем, я бы не стал его использовать.

это - UB.

Почему?

Вот так не знал. А есть хоть один пример такой глобальной переменной?

Зато автор знает что "t" - стандартное имя для аргумента/веса интерполяции

Не вижу как здесь применить switch. А хоть бы и можно, для 3 веток и if/else совсем неплох.

Вряд ли для членов класса (или локальных переменных - видел и так) это чем-то грозит. Иначе бы быстро полилось "из всех щелей".

Пока не вижу ничего кроме мелочных придирок. Мне вот не нравится отсутствие пробелов до/после операторов, ну что же, человек имеет право на свой стиль. Но код имеет не только "внешний вид", но и содержание, и что-то об этом пока не услышал ни слова.

А почему Вы полагаете что вообще есть выбор использовать "это" или "то"? Что у Вас под рукой будет что-то типа Qt вылизанное не раз и, в конце-концов, Вы уверенно раскрасите кнопку каким-нибудь делегатом?  :)  В жизни часто бывает совсем иначе

Перефразируя известную амерскую поговорку, "if it looks like a shit then it is a shit" :)
Другими словами, какой смысл от использования чужого кривого кода, который непонятно, что делает? :)

Насчет свитча - ну, там идея такая:

Если для канала чисел 3 и меньше - обрабатывать так-то. Что тут странного? Плохого? Почему нельзя использовать общее правило а надо непременно лупить свитчом по всем типам?

А почему кривой/говнистый? Только потому что "в нашем (UI-шном) муравейнике так не пишут!!!"?  Так это не показатель :)

Есть "каналы" (порядка 100-200), в каждом записано значения для N кадров. Задача (всего-навсего) интeрполировать значения каналов для заданного времени t. Разумеется каналы разных типов интерполируются по-разному. Плюс еще 2 наворота: во-первых некоторые каналы могут быть неактивны (map_p), Во-вторых записанные значения могут соответствовать другим исходным данным и интерполированные значения надо преобразовать для текущих (retarget)

Начинающий конечно такое не потянет - опыта маловато. А у "более опытных" тот опыт срабатывает в минус - "фырканья" много, а толку тот же ноль. Спросишь шо такое кватернион - лупает глазками. Может он и знает много - но ничего из того что нужно.

смысл в том, что стандарт декларирует собственную нотацию именования своих имен.
и объявляет UB (то есть последствия на совести разработчика),
если кто-то в своем коде использовал туже самую нотацию.

пример:
http://rextester.com/SZZHT85689


вывод:

но строго говоря это - пример простейшей коллизии с глобальными именами,
которые могут происходить в любой нотации при неаккуратном использовании.

другое дело, что согласно стандарту в "глобальные имена" включаются не только
простые имена переменных или функций,
но и макросы тоже.
а это - глобальные идентификаторы,
которым неведомы никакие барьеры.

а это уже означает, что если вы используете нотацию "реализации с++",
то у вас потенциально может возникнуть коллизия,
от которой вы никак не сможете превентивно защититься.

собственно, поэтому стандарт и объявляет посягательство на его нотацию как UB

грамотные любители подчеркиваний используют подчеркивания в конце имени,
но никак не в начале.